Merge pull request #1498 from sontek/allow_class_method_directly

Alert the user if they are using an unbound class methods to `add_view`

4 files changed, 116 insertions, 42 deletions

diff --git a/pyramid/compat.py b/pyramid/compat.py
index c49ea1e73..3aa191968 100644
--- a/pyramid/compat.py
+++ b/pyramid/compat.py
@@ -3,27 +3,27 @@ import platform
 import sys
 import types
 
-if platform.system() == 'Windows': # pragma: no cover
+if platform.system() == 'Windows':  # pragma: no cover
     WIN = True
-else: # pragma: no cover
+else:  # pragma: no cover
     WIN = False
 
-try: # pragma: no cover
+try:  # pragma: no cover
     import __pypy__
     PYPY = True
-except: # pragma: no cover
+except:  # pragma: no cover
     __pypy__ = None
     PYPY = False
 
 try:
     import cPickle as pickle
-except ImportError: # pragma: no cover
+except ImportError:  # pragma: no cover
     import pickle
 
 # True if we are running on Python 3.
 PY3 = sys.version_info[0] == 3
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     string_types = str,
     integer_types = int,
     class_types = type,
@@ -38,21 +38,23 @@ else:
     binary_type = str
     long = long
 
+
 def text_(s, encoding='latin-1', errors='strict'):
     """ If ``s`` is an instance of ``binary_type``, return
     ``s.decode(encoding, errors)``, otherwise return ``s``"""
     if isinstance(s, binary_type):
         return s.decode(encoding, errors)
-    return s # pragma: no cover
+    return s  # pragma: no cover
+
 
 def bytes_(s, encoding='latin-1', errors='strict'):
     """ If ``s`` is an instance of ``text_type``, return
     ``s.encode(encoding, errors)``, otherwise return ``s``"""
-    if isinstance(s, text_type): # pragma: no cover
+    if isinstance(s, text_type):  # pragma: no cover
         return s.encode(encoding, errors)
     return s
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     def ascii_native_(s):
         if isinstance(s, text_type):
             s = s.encode('ascii')
@@ -72,7 +74,7 @@ Python 2: If ``s`` is an instance of ``text_type``, return
 """
 
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     def native_(s, encoding='latin-1', errors='strict'):
         """ If ``s`` is an instance of ``text_type``, return
         ``s``, otherwise return ``str(s, encoding, errors)``"""
@@ -95,7 +97,7 @@ Python 2: If ``s`` is an instance of ``text_type``, return
 ``s.encode(encoding, errors)``, otherwise return ``str(s)``
 """
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     from urllib import parse
     urlparse = parse
     from urllib.parse import quote as url_quote
@@ -112,18 +114,19 @@ else:
     from urllib import unquote as url_unquote
     from urllib import urlencode as url_encode
     from urllib2 import urlopen as url_open
+
     def url_unquote_text(v, encoding='utf-8', errors='replace'): # pragma: no cover
         v = url_unquote(v)
         return v.decode(encoding, errors)
+
     def url_unquote_native(v, encoding='utf-8', errors='replace'): # pragma: no cover
         return native_(url_unquote_text(v, encoding, errors))
-        
 
-if PY3: # pragma: no cover
+
+if PY3:  # pragma: no cover
     import builtins
     exec_ = getattr(builtins, "exec")
 
-
     def reraise(tp, value, tb=None):
         if value is None:
             value = tp
@@ -131,10 +134,9 @@ if PY3: # pragma: no cover
             raise value.with_traceback(tb)
         raise value
 
-
     del builtins
 
-else: # pragma: no cover
+else:  # pragma: no cover
     def exec_(code, globs=None, locs=None):
         """Execute code in a namespace."""
         if globs is None:
@@ -147,35 +149,38 @@ else: # pragma: no cover
             locs = globs
         exec("""exec code in globs, locs""")
 
-
     exec_("""def reraise(tp, value, tb=None):
     raise tp, value, tb
 """)
 
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     def iteritems_(d):
         return d.items()
+
     def itervalues_(d):
         return d.values()
+
     def iterkeys_(d):
         return d.keys()
-else: # pragma: no cover
+else:  # pragma: no cover
     def iteritems_(d):
         return d.iteritems()
+
     def itervalues_(d):
         return d.itervalues()
+
     def iterkeys_(d):
         return d.iterkeys()
 
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     def map_(*arg):
         return list(map(*arg))
 else:
     map_ = map
-    
-if PY3: # pragma: no cover
+
+if PY3:  # pragma: no cover
     def is_nonstr_iter(v):
         if isinstance(v, str):
             return False
@@ -183,46 +188,52 @@ if PY3: # pragma: no cover
 else:
     def is_nonstr_iter(v):
         return hasattr(v, '__iter__')
-    
-if PY3: # pragma: no cover
+
+if PY3:  # pragma: no cover
     im_func = '__func__'
     im_self = '__self__'
 else:
     im_func = 'im_func'
     im_self = 'im_self'
 
-try: # pragma: no cover
+try:  # pragma: no cover
     import configparser
-except ImportError: # pragma: no cover
+except ImportError:  # pragma: no cover
     import ConfigParser as configparser
 
 try:
     from Cookie import SimpleCookie
-except ImportError: # pragma: no cover
+except ImportError:  # pragma: no cover
     from http.cookies import SimpleCookie
 
-if PY3: # pragma: no cover
+if PY3:  # pragma: no cover
     from html import escape
 else:
     from cgi import escape
 
-try: # pragma: no cover
+try:  # pragma: no cover
     input_ = raw_input
-except NameError: # pragma: no cover
+except NameError:  # pragma: no cover
     input_ = input
 
 
-try: 
+# support annotations and keyword-only arguments in PY3
+if PY3:  # pragma: no cover
+    from inspect import getfullargspec as getargspec
+else:
+    from inspect import getargspec
+
+try:
     from StringIO import StringIO as NativeIO
-except ImportError: # pragma: no cover
+except ImportError:  # pragma: no cover
     from io import StringIO as NativeIO
 
 # "json" is not an API; it's here to support older pyramid_debugtoolbar
 # versions which attempt to import it
 import json
 
-    
-if PY3: # pragma: no cover
+
+if PY3:  # pragma: no cover
     # see PEP 3333 for why we encode WSGI PATH_INFO to latin-1 before
     # decoding it to utf-8
     def decode_path_info(path):
@@ -231,21 +242,37 @@ else:
     def decode_path_info(path):
         return path.decode('utf-8')
 
-if PY3: # pragma: no cover
-    # see PEP 3333 for why we decode the path to latin-1 
+if PY3:  # pragma: no cover
+    # see PEP 3333 for why we decode the path to latin-1
     from urllib.parse import unquote_to_bytes
+
     def unquote_bytes_to_wsgi(bytestring):
         return unquote_to_bytes(bytestring).decode('latin-1')
 else:
     from urlparse import unquote as unquote_to_bytes
+
     def unquote_bytes_to_wsgi(bytestring):
         return unquote_to_bytes(bytestring)
 
+
 def is_bound_method(ob):
     return inspect.ismethod(ob) and getattr(ob, im_self, None) is not None
 
-# support annotations and keyword-only arguments in PY3
-if PY3: # pragma: no cover
-    from inspect import getfullargspec as getargspec
-else:
-    from inspect import getargspec
+
+def is_unbound_method(fn):
+    """
+    This consistently verifies that the callable is bound to a
+    class.
+    """
+    is_bound = is_bound_method(fn)
+
+    if not is_bound and inspect.isroutine(fn):
+        spec = inspect.getargspec(fn)
+        has_self = len(spec.args) > 0 and spec.args[0] == 'self'
+
+        if PY3 and inspect.isfunction(fn) and has_self:  # pragma: no cover
+            return True
+        elif inspect.ismethod(fn):
+            return True
+
+    return False
diff --git a/pyramid/config/views.py b/pyramid/config/views.py
index 1f69d7e0b..85e252f2f 100644
--- a/pyramid/config/views.py
+++ b/pyramid/config/views.py
@@ -42,7 +42,8 @@ from pyramid.compat import (
     url_quote,
     WIN,
     is_bound_method,
-    is_nonstr_iter
+    is_unbound_method,
+    is_nonstr_iter,
     )
 
 from pyramid.exceptions import (
@@ -418,6 +419,12 @@ class DefaultViewMapper(object):
         self.attr = kw.get('attr')
 
     def __call__(self, view):
+        if is_unbound_method(view) and self.attr is None:
+            raise ConfigurationError((
+                'Unbound method calls are not supported, please set the class '
+                'as your `view` and the method as your `attr`'
+            ))
+
         if inspect.isclass(view):
             view = self.map_class(view)
         else:
diff --git a/pyramid/tests/test_compat.py b/pyramid/tests/test_compat.py
new file mode 100644
index 000000000..23ccce82e
--- /dev/null
+++ b/pyramid/tests/test_compat.py
@@ -0,0 +1,26 @@
+import unittest
+from pyramid.compat import is_unbound_method
+
+class TestUnboundMethods(unittest.TestCase):
+    def test_old_style_bound(self):
+        self.assertFalse(is_unbound_method(OldStyle().run))
+
+    def test_new_style_bound(self):
+        self.assertFalse(is_unbound_method(NewStyle().run))
+
+    def test_old_style_unbound(self):
+        self.assertTrue(is_unbound_method(OldStyle.run))
+
+    def test_new_style_unbound(self):
+        self.assertTrue(is_unbound_method(NewStyle.run))
+
+    def test_normal_func_unbound(self):
+        def func(): return 'OK'
+
+        self.assertFalse(is_unbound_method(func))
+
+class OldStyle:
+    def run(self): return 'OK'
+
+class NewStyle(object):
+    def run(self): return 'OK'
diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py
index b0d03fb72..d1eb1ed3c 100644
--- a/pyramid/tests/test_config/test_views.py
+++ b/pyramid/tests/test_config/test_views.py
@@ -1666,6 +1666,20 @@ class TestViewsConfigurationMixin(unittest.TestCase):
             renderer=null_renderer)
         self.assertRaises(ConfigurationConflictError, config.commit)
 
+    def test_add_view_class_method_no_attr(self):
+        from pyramid.renderers import null_renderer
+        from zope.interface import directlyProvides
+        from pyramid.exceptions import ConfigurationError
+
+        config = self._makeOne(autocommit=True)
+        class DummyViewClass(object):
+            def run(self): pass
+
+        def configure_view():
+            config.add_view(view=DummyViewClass.run, renderer=null_renderer)
+
+        self.assertRaises(ConfigurationError, configure_view)
+
     def test_derive_view_function(self):
         from pyramid.renderers import null_renderer
         def view(request):