Author: Carl Friedrich Bolz <cfb...@gmx.de>
Branch: py3k
Changeset: r87586:c0c28e08bb1c
Date: 2016-10-04 17:53 +0200
http://bitbucket.org/pypy/pypy/changeset/c0c28e08bb1c/

Log:    merge default

        (the methodcall stuff needed a slightly different approach than on
        default)

diff --git a/pypy/doc/whatsnew-head.rst b/pypy/doc/whatsnew-head.rst
--- a/pypy/doc/whatsnew-head.rst
+++ b/pypy/doc/whatsnew-head.rst
@@ -57,3 +57,7 @@
 .. branch: test-cpyext
 
 Refactor cpyext testing to be more pypy3-friendly.
+
+.. branch: better-error-missing-self
+
+Improve the error message when the user forgot the "self" argument of a method.
diff --git a/pypy/interpreter/argument.py b/pypy/interpreter/argument.py
--- a/pypy/interpreter/argument.py
+++ b/pypy/interpreter/argument.py
@@ -23,7 +23,8 @@
     ###  Construction  ###
     #@enforceargs(keywords=[unicode])
     def __init__(self, space, args_w, keywords=None, keywords_w=None,
-                 w_stararg=None, w_starstararg=None, keyword_names_w=None):
+                 w_stararg=None, w_starstararg=None, keyword_names_w=None,
+                 methodcall=False):
         self.space = space
         assert isinstance(args_w, list)
         self.arguments_w = args_w
@@ -43,6 +44,9 @@
         # a flag that specifies whether the JIT can unroll loops that operate
         # on the keywords
         self._jit_few_keywords = self.keywords is None or 
jit.isconstant(len(self.keywords))
+        # a flag whether this is likely a method call, which doesn't change the
+        # behaviour but produces better error messages
+        self.methodcall = methodcall
 
     def __repr__(self):
         """ NOT_RPYTHON """
@@ -270,7 +274,11 @@
             for i in range(co_argcount, co_argcount + co_kwonlyargcount):
                 if scope_w[i] is not None:
                     kwonly_given += 1
-            raise ArgErrTooMany(signature.num_argnames(),
+            if self.methodcall:
+                cls = ArgErrTooManyMethod
+            else:
+                cls = ArgErrTooMany
+            raise cls(signature,
                                 0 if defaults_w is None else len(defaults_w),
                                 avail, kwonly_given)
 
@@ -498,14 +506,14 @@
 
 
 class ArgErrTooMany(ArgErr):
-    def __init__(self, num_args, num_defaults, given, kwonly_given):
-        self.num_args = num_args
+    def __init__(self, signature, num_defaults, given, kwonly_given):
+        self.signature = signature
         self.num_defaults = num_defaults
         self.given = given
         self.kwonly_given = kwonly_given
 
     def getmsg(self):
-        num_args = self.num_args
+        num_args = self.signature.num_argnames()
         num_defaults = self.num_defaults
         if num_defaults:
             takes_str = "from %d to %d positional arguments" % (
@@ -524,6 +532,22 @@
         msg = "takes %s but %s given" % (takes_str, given_str)
         return msg
 
+class ArgErrTooManyMethod(ArgErrTooMany):
+    """ A subclass of ArgErrCount that is used if the argument matching is done
+    as part of a method call, in which case more information is added to the
+    error message, if the cause of the error is likely a forgotten `self`
+    argument.
+    """
+
+    def getmsg(self):
+        msg = ArgErrTooMany.getmsg(self)
+        n = self.signature.num_argnames()
+        if (self.given == n + 1 and
+                (n == 0 or self.signature.argnames[0] != "self")):
+            msg += ". Did you forget 'self' in the function definition?"
+        return msg
+
+
 class ArgErrMultipleValues(ArgErr):
 
     def __init__(self, argname):
diff --git a/pypy/interpreter/baseobjspace.py b/pypy/interpreter/baseobjspace.py
--- a/pypy/interpreter/baseobjspace.py
+++ b/pypy/interpreter/baseobjspace.py
@@ -1117,7 +1117,8 @@
         args = Arguments(self, list(args_w))
         return self.call_args(w_func, args)
 
-    def call_valuestack(self, w_func, nargs, frame):
+    def call_valuestack(self, w_func, nargs, frame, methodcall=False):
+        # methodcall is only used for better error messages in argument.py
         from pypy.interpreter.function import Function, Method, is_builtin_code
         if frame.get_is_being_profiled() and is_builtin_code(w_func):
             # XXX: this code is copied&pasted :-( from the slow path below
@@ -1131,10 +1132,12 @@
                 # reuse callable stack place for w_inst
                 frame.settopvalue(w_func.w_instance, nargs)
                 nargs += 1
+                methodcall = True
                 w_func = w_func.w_function
 
             if isinstance(w_func, Function):
-                return w_func.funccall_valuestack(nargs, frame)
+                return w_func.funccall_valuestack(
+                        nargs, frame, methodcall=methodcall)
             # end of hack for performance
 
         args = frame.make_arguments(nargs)
diff --git a/pypy/interpreter/function.py b/pypy/interpreter/function.py
--- a/pypy/interpreter/function.py
+++ b/pypy/interpreter/function.py
@@ -125,7 +125,8 @@
                                               list(args_w[1:])))
         return self.call_args(Arguments(self.space, list(args_w)))
 
-    def funccall_valuestack(self, nargs, frame): # speed hack
+    def funccall_valuestack(self, nargs, frame, methodcall=False): # speed hack
+        # methodcall is only for better error messages
         from pypy.interpreter import gateway
         from pypy.interpreter.pycode import PyCode
 
@@ -172,7 +173,7 @@
             args = frame.make_arguments(nargs-1)
             return code.funcrun_obj(self, w_obj, args)
 
-        args = frame.make_arguments(nargs)
+        args = frame.make_arguments(nargs, methodcall=methodcall)
         return self.call_args(args)
 
     @jit.unroll_safe
diff --git a/pypy/interpreter/pyframe.py b/pypy/interpreter/pyframe.py
--- a/pypy/interpreter/pyframe.py
+++ b/pypy/interpreter/pyframe.py
@@ -403,11 +403,14 @@
             depth -= 1
         self.valuestackdepth = finaldepth
 
-    def make_arguments(self, nargs):
-        return Arguments(self.space, self.peekvalues(nargs))
+    def make_arguments(self, nargs, methodcall=False):
+        return Arguments(
+                self.space, self.peekvalues(nargs), methodcall=methodcall)
 
-    def argument_factory(self, arguments, keywords, keywords_w, w_star, 
w_starstar):
-        return Arguments(self.space, arguments, keywords, keywords_w, w_star, 
w_starstar)
+    def argument_factory(self, arguments, keywords, keywords_w, w_star, 
w_starstar, methodcall=False):
+        return Arguments(
+                self.space, arguments, keywords, keywords_w, w_star,
+                w_starstar, methodcall=methodcall)
 
     @jit.dont_look_inside
     def descr__reduce__(self, space):
diff --git a/pypy/interpreter/test/test_argument.py 
b/pypy/interpreter/test/test_argument.py
--- a/pypy/interpreter/test/test_argument.py
+++ b/pypy/interpreter/test/test_argument.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 import py
 from pypy.interpreter.argument import (Arguments, ArgErr, ArgErrUnknownKwds,
-        ArgErrMultipleValues, ArgErrMissing, ArgErrTooMany)
+        ArgErrMultipleValues, ArgErrMissing, ArgErrTooMany, 
ArgErrTooManyMethod)
 from pypy.interpreter.signature import Signature
 from pypy.interpreter.error import OperationError
 
@@ -585,38 +585,82 @@
         assert s == "missing 1 required keyword-only argument: 'a'"
 
     def test_too_many(self):
-        err = ArgErrTooMany(0, 0, 1, 0)
+        sig0 = Signature([], None, None)
+        err = ArgErrTooMany(sig0, 0, 1, 0)
         s = err.getmsg()
         assert s == "takes 0 positional arguments but 1 was given"
 
-        err = ArgErrTooMany(0, 0, 2, 0)
+        err = ArgErrTooMany(sig0, 0, 2, 0)
         s = err.getmsg()
         assert s == "takes 0 positional arguments but 2 were given"
 
-        err = ArgErrTooMany(1, 0, 2, 0)
+        sig1 = Signature(['a'], None, None)
+        err = ArgErrTooMany(sig1, 0, 2, 0)
         s = err.getmsg()
         assert s == "takes 1 positional argument but 2 were given"
 
-        err = ArgErrTooMany(2, 0, 3, 0)
+        sig2 = Signature(['a', 'b'], None, None)
+        err = ArgErrTooMany(sig2, 0, 3, 0)
         s = err.getmsg()
         assert s == "takes 2 positional arguments but 3 were given"
 
-        err = ArgErrTooMany(2, 1, 3, 0)
+        err = ArgErrTooMany(sig2, 1, 3, 0)
         s = err.getmsg()
         assert s == "takes from 1 to 2 positional arguments but 3 were given"
 
-        err = ArgErrTooMany(0, 0, 1, 1)
+        err = ArgErrTooMany(sig0, 0, 1, 1)
         s = err.getmsg()
         assert s == "takes 0 positional arguments but 1 positional argument 
(and 1 keyword-only argument) were given"
 
-        err = ArgErrTooMany(0, 0, 2, 1)
+        err = ArgErrTooMany(sig0, 0, 2, 1)
         s = err.getmsg()
         assert s == "takes 0 positional arguments but 2 positional arguments 
(and 1 keyword-only argument) were given"
 
-        err = ArgErrTooMany(0, 0, 1, 2)
+        err = ArgErrTooMany(sig0, 0, 1, 2)
         s = err.getmsg()
         assert s == "takes 0 positional arguments but 1 positional argument 
(and 2 keyword-only arguments) were given"
 
+    def test_too_many_method(self):
+        sig0 = Signature([], None, None)
+        err = ArgErrTooManyMethod(sig0, 0, 1, 0)
+        s = err.getmsg()
+        assert s == "takes 0 positional arguments but 1 was given. Did you 
forget 'self' in the function definition?"
+
+        err = ArgErrTooManyMethod(sig0, 0, 2, 0)
+        s = err.getmsg()
+        assert s == "takes 0 positional arguments but 2 were given"
+
+        sig1 = Signature(['self'], None, None)
+        err = ArgErrTooManyMethod(sig1, 0, 2, 0)
+        s = err.getmsg()
+        assert s == "takes 1 positional argument but 2 were given"
+
+        sig1 = Signature(['a'], None, None)
+        err = ArgErrTooManyMethod(sig1, 0, 2, 0)
+        s = err.getmsg()
+        assert s == "takes 1 positional argument but 2 were given. Did you 
forget 'self' in the function definition?"
+
+        sig2 = Signature(['a', 'b'], None, None)
+        err = ArgErrTooManyMethod(sig2, 0, 3, 0)
+        s = err.getmsg()
+        assert s == "takes 2 positional arguments but 3 were given. Did you 
forget 'self' in the function definition?"
+
+        err = ArgErrTooManyMethod(sig2, 1, 3, 0)
+        s = err.getmsg()
+        assert s == "takes from 1 to 2 positional arguments but 3 were given. 
Did you forget 'self' in the function definition?"
+
+        err = ArgErrTooManyMethod(sig0, 0, 1, 1)
+        s = err.getmsg()
+        assert s == "takes 0 positional arguments but 1 positional argument 
(and 1 keyword-only argument) were given. Did you forget 'self' in the function 
definition?"
+
+        err = ArgErrTooManyMethod(sig0, 0, 2, 1)
+        s = err.getmsg()
+        assert s == "takes 0 positional arguments but 2 positional arguments 
(and 1 keyword-only argument) were given"
+
+        err = ArgErrTooManyMethod(sig0, 0, 1, 2)
+        s = err.getmsg()
+        assert s == "takes 0 positional arguments but 1 positional argument 
(and 2 keyword-only arguments) were given. Did you forget 'self' in the 
function definition?"
+
     def test_bad_type_for_star(self):
         space = self.space
         try:
@@ -748,6 +792,45 @@
         exc = raises(TypeError, '(lambda *, kw: 0)(1, kw=3)')
         assert str(exc.value) == "<lambda>() takes 0 positional arguments but 
1 positional argument (and 1 keyword-only argument) were given"
 
+    @py.test.mark.skipif("config.option.runappdirect")
+    def test_error_message_method(self):
+        class A(object):
+            def f0():
+                pass
+            def f1(a):
+                pass
+        exc = raises(TypeError, lambda : A().f0())
+        assert exc.value.args[0] == "f0() takes 0 positional arguments but 1 
was given. Did you forget 'self' in the function definition?"
+        exc = raises(TypeError, lambda : A().f1(1))
+        assert exc.value.args[0] == "f1() takes 1 positional argument but 2 
were given. Did you forget 'self' in the function definition?"
+        def f0():
+            pass
+        exc = raises(TypeError, f0, 1)
+        # does not contain the warning about missing self
+        assert exc.value.args[0] == "f0() takes 0 positional arguments but 1 
was given"
+
+    @py.test.mark.skipif("config.option.runappdirect")
+    def test_error_message_module_function(self):
+        import operator # use countOf because it's defined at applevel
+        exc = raises(TypeError, lambda : operator.countOf(1, 2, 3))
+        # does not contain the warning about missing self
+        assert exc.value.args[0] == "countOf() takes 2 positional arguments 
but 3 were given"
+
+    @py.test.mark.skipif("config.option.runappdirect")
+    def test_error_message_bound_method(self):
+        class A(object):
+            def f0():
+                pass
+            def f1(a):
+                pass
+        m0 = A().f0
+        exc = raises(TypeError, lambda : m0())
+        assert exc.value.args[0] == "f0() takes 0 positional arguments but 1 
was given. Did you forget 'self' in the function definition?"
+        m1 = A().f1
+        exc = raises(TypeError, lambda : m1(1))
+        assert exc.value.args[0] == "f1() takes 1 positional argument but 2 
were given. Did you forget 'self' in the function definition?"
+
+
     def test_unicode_keywords(self):
         """
         def f(**kwargs):
diff --git a/pypy/module/pypyjit/test_pypy_c/test_call.py 
b/pypy/module/pypyjit/test_pypy_c/test_call.py
--- a/pypy/module/pypyjit/test_pypy_c/test_call.py
+++ b/pypy/module/pypyjit/test_pypy_c/test_call.py
@@ -388,6 +388,7 @@
             setfield_gc(p22, ConstPtr(null), descr=<FieldP 
pypy.interpreter.argument.Arguments.inst_keywords_w .*>)
             setfield_gc(p22, ConstPtr(null), descr=<FieldP 
pypy.interpreter.argument.Arguments.inst_keywords .*>)
             setfield_gc(p22, 1, descr=<FieldU 
pypy.interpreter.argument.Arguments.inst__jit_few_keywords .*>)
+            setfield_gc(p22, 0, descr=<FieldU 
pypy.interpreter.argument.Arguments.inst_methodcall .*>)
             setfield_gc(p22, ConstPtr(null), descr=<FieldP 
pypy.interpreter.argument.Arguments.inst_keyword_names_w .*>)
             setfield_gc(p26, ConstPtr(ptr22), descr=<FieldP 
pypy.objspace.std.listobject.W_ListObject.inst_strategy .*>)
             setfield_gc(p26, ConstPtr(null), descr=<FieldP 
pypy.objspace.std.listobject.W_ListObject.inst_lstorage .*>)
diff --git a/pypy/objspace/std/callmethod.py b/pypy/objspace/std/callmethod.py
--- a/pypy/objspace/std/callmethod.py
+++ b/pypy/objspace/std/callmethod.py
@@ -93,7 +93,8 @@
     if not n_kwargs:
         w_callable = f.peekvalue(n_args + (2 * n_kwargs) + 1)
         try:
-            w_result = f.space.call_valuestack(w_callable, n, f)
+            w_result = f.space.call_valuestack(
+                    w_callable, n, f, methodcall=w_self is not None)
         finally:
             f.dropvalues(n_args + 2)
     else:
@@ -110,7 +111,9 @@
             keywords_w[n_kwargs] = w_value
 
         arguments = f.popvalues(n)    # includes w_self if it is not None
-        args = f.argument_factory(arguments, keywords, keywords_w, None, None)
+        args = f.argument_factory(
+                arguments, keywords, keywords_w, None, None,
+                methodcall=w_self is not None)
         if w_self is None:
             f.popvalue()    # removes w_self, which is None
         w_callable = f.popvalue()
_______________________________________________
pypy-commit mailing list
pypy-commit@python.org
https://mail.python.org/mailman/listinfo/pypy-commit

Reply via email to