classification
Title: IDLE: calltips mishandle raw strings and other examples
Type: behavior Stage: resolved
Components: IDLE Versions: Python 3.3, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: Roy.Fox, ned.deily, python-dev, roger.serwy, terry.reedy
Priority: normal Keywords: patch

Created on 2011-07-06 23:27 by Roy.Fox, last changed 2012-12-12 01:33 by roger.serwy. This issue is now closed.

Files
File name Uploaded Description Edit
issue12510.patch roger.serwy, 2011-12-21 18:37 review
issue12510_rev1.patch roger.serwy, 2012-05-27 20:19 review
fix_12510.patch roger.serwy, 2012-06-03 21:27 review
i12510.test.diff terry.reedy, 2012-06-04 03:01 Improve get_argspec and tests. review
i12510b.test.diff terry.reedy, 2012-06-05 09:21 Improve get_argspec and tests.
i12510c.test.diff terry.reedy, 2012-06-05 21:06 Improve get_argspec and tests. review
issue12510_self_pat.patch roger.serwy, 2012-06-12 19:33 review
Messages (29)
msg139961 - (view) Author: Roy Fox (Roy.Fox) Date: 2011-07-06 23:27
Hi,

When you type (not copy-paste!) into an IDLE shell a string literal followed by (
you get a calltip.

When the string contains a bad unicode escaping you get an error (see example below), which makes some sense.

But when the string is raw, it isn't treated as such, and you may get the same error, though now it doesn't make any sense.

Non-raw example:

>>> '\xyz'(

>>> *** Internal Error: rpc.py:SocketIO.localcall()

 Object: exec 
 Method: <bound method Executive.get_the_calltip of <idlelib.run.Executive object at 0x021C2910>> 
 Args: ("'\\xyz'",)

Traceback (most recent call last):
  File "C:\Python32\lib\idlelib\rpc.py", line 188, in localcall
    ret = method(*args, **kwargs)
  File "C:\Python32\lib\idlelib\run.py", line 324, in get_the_calltip
    return self.calltip.fetch_tip(name)
  File "C:\Python32\lib\idlelib\CallTips.py", line 103, in fetch_tip
    entity = self.get_entity(name)
  File "C:\Python32\lib\idlelib\CallTips.py", line 112, in get_entity
    return eval(name, namespace)
  File "<string>", line 1
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 0-2: truncated \xXX escape


Raw example:

>>> r'\xyz'(

>>> *** Internal Error: rpc.py:SocketIO.localcall()

 Object: exec 
 Method: <bound method Executive.get_the_calltip of <idlelib.run.Executive object at 0x021C2910>> 
 Args: ("'\\xyz'",)

Traceback (most recent call last):
  File "C:\Python32\lib\idlelib\rpc.py", line 188, in localcall
    ret = method(*args, **kwargs)
  File "C:\Python32\lib\idlelib\run.py", line 324, in get_the_calltip
    return self.calltip.fetch_tip(name)
  File "C:\Python32\lib\idlelib\CallTips.py", line 103, in fetch_tip
    entity = self.get_entity(name)
  File "C:\Python32\lib\idlelib\CallTips.py", line 112, in get_entity
    return eval(name, namespace)
  File "<string>", line 1
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 0-2: truncated \xXX escape
msg140051 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-07-09 05:42
What platform? It sometimes makes a difference with tcl/tk and hence idle.
msg140056 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2011-07-09 09:31
The problem is easily reproducible.  Although it shouldn't give that error (and that can be fixed), it seems to me that IDLE should not be trying to give a calltip in that context. What it is trying to do is display the __doc__ attribute of the string but the __doc__ is really for the str() constructor:

>>> 'a'.__doc__
"str(string[, encoding[, errors]]) -> str\n\nCreate a new string object from the given encoded string.\nencoding defaults to the current default string encoding.\nerrors can be 'strict', 'replace' or 'ignore' and defaults to 'strict'."
msg150033 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2011-12-21 18:37
The attached patch fixes the bug. The bug occurs in "get_entity" which is used to get the object. Then "get_argspec" determines the calltip text. 

The calltip can be prevented for strings by having get_argspec check if the object has a "__call__" method. I opted for adding "not callable" to the calltip initially for testing, but setting argspec="" will prevent the calltip altogether.
msg161698 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-05-27 07:31
With 2.7 & 3.3 on Win 7, 'a'( brings up calltip (bad).
With 3.3, '\xyz'( causes Idle to die (worse)
With 2.7, the same or u'\xyz'( have no effect, just like 3(.

There are a lot of little differences between 2.7 and 3.x, but the main one is the use of inspect in 3.x. I do not see an obvious reason for the behavior difference.

With patch, 'a'( brings up calltip with 'Not callable' added (worse, in my opinion), while '\xyz'( does nothing, just like 3( (or 2.7, great).

Roger, perhaps you uploaded your initial test past instead of the final patch. A string instance should act like any other non-callable instance.

That said, the patch strikes me as attacking the problem at the wrong end. Str instances are not names and get_argspec should never be called on one (I am presuming that it is not called on other non-callables, such as int instances). Similarly, back up further in the call stack, fetch_tip(name) should only be called on names. I wonder if the problem might be back up in open_calltip(), perhaps after

        hp = HyperParser(self.editwin, "insert")
        sur_paren = hp.get_surrounding_brackets('(')
        if not sur_paren:
            return
        hp.set_index(sur_paren[0])
        name = hp.get_expression()

(argggh! I have no idea how to debug code where I cannot just insert print statements to tell me what is getting called with what arguments.)
msg161721 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-05-27 20:19
Terry, the original patch added "Not callable" because I wasn't sure what the consensus was on proper behavior. Now I know. :)

Attached is a revision against 3.3a3+ which omits the calltip if the object is not callable.

The behavior difference between 2.7 and 3.3 is due to a difference in "localcall" in rpc.py, where "CALLEXC" raises the exception in the IDLE front-end in 3.3 causing the crash, whereas in 2.7 the exception occurs in the subprocess. This is due to "rpc_marshal_exception.patch" for issue14200 not being backported to 2.7.

Entering []( or {}( or ''( will bring up a calltip. These calltips should not appear if the object is not callable. Take as an example:

    a = "string"
    a(

The previous code will bring up a calltip even though "a" is not callable. The HyperParser does its job by returning the string representing the object just before the paren. Checking whether this is a proper Python name won't fix this corner case. The "fetch_tip" method grabs the actual object given the string name using "get_entity" and then uses "get_argspec" to return constructor or callable arguments.

Checking if the object is callable must be done either in "get_entity" or in "get_argspec". I think it is cleaner to check in get_argspec, as is done in the attached patch.
msg161742 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-05-28 00:10
'a'(, [](, {}( no longer do anything.
[int][0]( and {0:int}[0] bring up int tooltip.

However, patch is not a complete fix yet.
[int][1]( and {0:int}[1] both 'crash' Idle.
So we need to add IndexError and KeyError to
+            except (NameError, AttributeError, SyntaxError):

I think instead we should just remove the list since any uncaught error in the eval will close Idle. Do you see any reason why not? If open_calltips were called with evalfuncs True (I presume there must be a way, see below), then *any* exception could happen in the eval.

I was thinking that eval is a bit dangerous but I see this protection in open_calltips"

        if not evalfuncs and (name.find('(') != -1):
            return

Consequently, after "def f(): return int", f()( does not get a calltip. Neither does the tuple equivalent of the list example above, (int,)[0](.

With the call check moved from the bottom of get_argspec to the top, I am ok with it there. If we agree on the except clause, I am ready to apply.
msg161743 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-05-28 00:20
I'm ok with removing the exception list, but add a comment like "# *any* exception could happen in the eval".
msg161748 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-28 01:55
New changeset 938b12452af7 by Terry Jan Reedy in branch '2.7':
Issue12510: Attempting to get invalid tooltip no longer closes Idle.
http://hg.python.org/cpython/rev/938b12452af7

New changeset 4a7582866735 by Terry Jan Reedy in branch '3.2':
Issue12510: Attempting to get invalid tooltip no longer closes Idle.
http://hg.python.org/cpython/rev/4a7582866735

New changeset 0835bee19f86 by Terry Jan Reedy in branch 'default':
Merge 3.2 closes #12510
http://hg.python.org/cpython/rev/0835bee19f86
msg161828 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-05-29 01:43
I am reopening just as a reminder to revise the comment.
msg161847 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-05-29 03:36
Which comment needs revision?
msg162192 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-03 05:07
New changeset 477508efe4ab by Terry Jan Reedy in branch '2.7':
Issue 12510: Expand 2 bare excepts. Improve comments. Change deceptive name
http://hg.python.org/cpython/rev/477508efe4ab

New changeset f927a5c6e4be by Terry Jan Reedy in branch '3.2':
Issue 12510: Expand 2 bare excepts. Improve comments. Change deceptive name
http://hg.python.org/cpython/rev/f927a5c6e4be

New changeset a7501ddf74ac by Terry Jan Reedy in branch 'default':
Merge with 3.2 #12510
http://hg.python.org/cpython/rev/a7501ddf74ac
msg162195 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-03 05:17
CallTips.py has a 'main' test at the end. Currently, test TC fails in 3.x but not 2.7. So either the test became invalid or get_argspec was not completely and correctly converted from get_arg_text. This should be fixed.

int.append( does not bring up a tip on either version, but should if possible.

The failing examples in previous messages should be added to the tests to ensure that all pass now and continue to pass in the future. So I am not closing this yet, and a new patch is needed.
msg162233 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-06-03 21:27
fix_12510.patch addresses the issue with the test.

What do you mean by: "int.append( does not bring up a tip on either version, but should if possible." ? The "int" object does not have "append" as a method.
msg162248 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-04 03:01
i12510.test.diff (for Python 3 only) does the following:

* Turn method CallTips.get_entity into a module function as it does not use the self parameter and is therefore not really an instance method.

* Delete the erroneous _find_constructor function. Even if it were fixed, it is unnecessary at least in 3.x. Class attribute lookup already does the superclass search duplicated in this function and should find object.__init__ if nothing else. (I defaulted to None anyway in case of an unforeseen problem, such as a type subclass that somehow disables the lookup.)

* In get_argspec, remove the lambda stuff that resulted in all classes getting a spurious '()' line. With the fix to retrieving .__init__, it never gets used and is not needed anyway.

* Only delete 'self' from the argument list for classes and bound methods where it is implicit and not to be explicity entered.

* Increase tests from 13 to 37.

* Modernize and re-organize tests with an eye to becoming part of an idlelib test suite.

With the increased number of tests, I am pretty confident in the patch, but I would not mind if you glance at it and try it out on Linux.
msg162324 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-06-05 03:09
The reoganization in i12510.test.diff will make back-porting to 2.7 slightly more difficult since old and new style classes exist. I do agree with your reasoning for re-factoring the code. (Also, as an extremely minor point, running reindent.py adjusts "pos = 70".)

I applied the patch and it works under 11.04 Ubuntu. All the tests pass and the behavior of the CallTip gives reasonable results when used interactively.

I did manage to find a corner-case where a CallTip should arise:
    >>> class A:
            def __init__(self, a=None):
                print('init')
            def __call__(self, b=None):
                print('call')
		
    >>> c = A(

Gives "(a=None)" as the call-tip. However, once the object is created, giving the __call__ argument doesn't work, i.e. 

    >>> c(

doesn't give a call-tip. This behavior is also the same without the patch.

I'll keep playing with the patch to see what else needs improving with CallTips. We might as well fix it completely.
msg162334 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-05 09:21
By 'Python 3 only', I meant that I am not personally planning to backport to 2.7. I do not want to edit the more complicated get_arg_text and consider other issues higher priority.
 
I meant to include a test case for callable instances, but forgot. When 
I add to test class TC
"        def __call__(self, ci): "(self, ci)"
tc( gives the inherited TC.__doc__ as the tool tip, which in this case happens to by the artificial "(ai=None, *b)".

Adding the following to get_argspec:

        # after current if is...type: fob = ...
        elif isinstance(ob.__call__, types.MethodType):
            fob = ob.__call__
&
             or isinstance(ob.__call__, types.MethodType)
        # to self-strip conditional, as instance is already typed 
&
        if isinstance(ob.__call__, types.MethodType):
            doc = ob.__call__.__doc__
        else:
          # before current doc = line, which gets indented

makes this new test pass.
        fdoc = tc.__call__.__doc__
        test('tc', fdoc + "\n" + fdoc)

Revised patch with above attached.

Note 1: Programming is more fun with decent tests.
Note 2: These non-visual automatic test do not completely substitute for visual testing with the gui. When I first used a broader conditional than the 'elif' above, one that caught list.append and [].append, the tests passed, but typing list.append( resulting in weird behavior: something flashed and the typed text moved down a line.
msg162335 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-05 09:40
Stephen D'Aprano just asked on Python list how to get signatures of builtins, noting that calling inspect.getargspec on a Python-coded subclass of a builtin class raises an exception. So we need another testcase, verify the behavior, and probably add try-except around the inspect call. (Perhaps inspect.py should be revised to not leak an exception when given the advertised acceptible input, but that is another issue.)
msg162336 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-05 09:55
Upon further thought, not a problem. Stephen considered the case:
class myint(int):
    # inherit __init__
    def added_func(self): "some new function of an int"

In this case, fob = myint.__init__ would not be FunctionType and inspect would not be called. Tool tip would be myint.__doc__ and writer could follow style of builtins. Still an interesting test case to add.

A different question is whether the default for callables (as opposed to non-callables) should be nothing or something like 'args unknown' or 'see docs'.
msg162353 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-06-05 16:03
Callable instances now return a call tip. Good!

I agree with your analysis of the issue raised by Stephen. The get_argspec function will not fail unexpectedly.

> but typing list.append( resulting in weird behavior: something flashed and the typed text moved down a line.

I get "L.append(object) -> None -- append object to end" as the call tip on Linux. I'm not sure why it didn't work on Win 7. Could an unintentional enter key-press be the culprit?


> A different question is whether the default for callables (as opposed to non-callables) should be nothing or something like 'args unknown' or 'see docs'.

I agree that if the argspec string has no contents after all the code in get_argspec, then it should have a message added. Perhaps the following would be sufficient:

    if hasattr(ob, '__call__'):

        [truncated]

        if not argspec:
            argspec += "Arguments unknown. See docs."
    return argspec
msg162373 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-05 21:06
The weird behavior was as if the cursor were moved to the beginning of the line and <Enter> pressed. I cannot duplicate it. I had 
        elif not isinstance(ob, (types.FunctionType, types.MethodType)):
instead
        elif isinstance(ob.__call__, types.MethodType):
but something else must have been different, as list.append( works now with above changed back. The tests pass because passing fob to inspect is guarded. But the correct conditional is needed anyway elsewhere in the file. So I will forget this until it happens again ;-).

Since we (intend to) inspect all Python-coded callables, and since that returns at least '()' for no args (there is already a test for that), empty callable argspec should only happen for builtin and extension callables with no docstring. I added this code:

_default_callable_argspec = "No docstring, see docs."
# before get_argspec

          if not argspec:
              argspec = _default_callable_argspec

I added tests for this and for subclasses of builtins and uploaded a new patch. Can you think of anything else to test?

Now that the code and test cases seem about settled, the other (big) change I would like to make is to make most of the tests into proper unittests of get_argspec (which means passing an object, not a string) and some (with some overlap) into unittests of get_entity (passing expression string, getting object). Since rpcclt is None when tests call CallTips.fetch_tip, the method reduces to get_argspec(get_entity(expression)), which just mixes tests of the two functions together.
msg162510 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-08 00:05
New changeset 02b4c62ce393 by Terry Jan Reedy in branch '3.2':
Issue #12510: Revise and triple # of calltip tests, with an eye to unittest
http://hg.python.org/cpython/rev/02b4c62ce393

New changeset 03b5f75ddac7 by Terry Jan Reedy in branch 'default':
Merge from 3.2, #12510
http://hg.python.org/cpython/rev/03b5f75ddac7
msg162511 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-08 00:14
I decided to commit the current patch (i12510c.test.diff + NEWS changes) as is. Another change is needed to correctly delete the first arg of all methods, not just instance methods with the first arg named 'self'. (In the current new classmethod test, I just allowed non-removal of 'cls'. There should also be a test with a variant of 'self'.)

In _self_pat = re.compile('self\,?\s*'), 'self' in the pattern should be replaced by '(xxx', where 'xxx' is the pattern that matches an identifier. (And add '(' to the replacement pattern.) If anyone can quickly tell me what that is, that would be helpful.
msg162690 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-06-12 19:33
The _self_pat RE needs to be changed to just remove the first argument. Presently, another bug exists with the current implementation:

    >>> class A:
            def t(self, self1, self2):
                pass
    >>> a = A()
    >>> a.t(

gives "(1,2)" as the calltip, instead of "(self1, self2)" for 3.x. Python 2.7 gives the correct calltip.

The attached patch modifies _self_pat to remove only the first argument, modifies the classmethod test, and adds a test for "notself", as Terry requested in msg162511.
msg165057 - (view) Author: Roundup Robot (python-dev) Date: 2012-07-09 04:15
New changeset eea379307efa by Terry Jan Reedy in branch '3.2':
Issue 12510: Delete actual first param name for all methods; revise tests.
http://hg.python.org/cpython/rev/eea379307efa

New changeset 464c6a50b0ce by Terry Jan Reedy in branch 'default':
Merge with 3.2 Issue 12510
http://hg.python.org/cpython/rev/464c6a50b0ce
msg165058 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-07-09 04:17
Thank you for the 'positive lookbehind assertion' for anchoring the match after '('. That is what I was missing. '[^,]' is too broad as, in particular, it matches the ')' in 'a(self)'. We don't want the ')' removed. The tests passed with a faulty re because the method test was faulty in that it only tested that something is removed, but not that the removal is correct. I fixed the tests also to really test proper removal.
msg165793 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-07-18 19:40
Are there any other aspects of the calltip handler that requires fixing? If not, then I can start back-porting the patches.
msg165822 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-07-19 01:16
The original patches were to all 3 versions. The later patches were to both 3.x versions and mostly involved get_argspec and expanding the tests. Get_argspec is somewhat different from get_arg_text, the 2.x function it replaced. The main difference I could see is that the latter attempt to deal with old-style classes (I think) and does not use inspect.

I *think* I am done changing get_argspec until I try making use of the new PEP 362 function signature objects, but that can only affect 3.3+. (This would be to improve calltips for builtins). So if you want to backport further changes to 2.7, so it can pass more of the expanded test suite, now is a good time. Since the TC test class in 2.7 is derived from object, there seems to be no test of old style classes, so I don't know if just replacing get_arg_text with get_argspec would break anything that actually works, assuming that get_argspec does work ;-).
msg177361 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-12-12 01:33
I haven't had any serious problems with calltips on the dev branches for 2.7 and 3.4, except for issue16630. I think this issue can be closed as being completed in order to avoid further expanding its scope. Feel free to reopen if anyone disagrees.
History
Date User Action Args
2012-12-12 01:33:41roger.serwysetstatus: open -> closed
resolution: fixed
messages: + msg177361

stage: patch review -> resolved
2012-07-19 01:16:10terry.reedysetmessages: + msg165822
2012-07-18 19:40:08roger.serwysetmessages: + msg165793
2012-07-09 04:17:31terry.reedysetmessages: + msg165058
2012-07-09 04:15:58python-devsetmessages: + msg165057
2012-06-12 19:33:52roger.serwysetfiles: + issue12510_self_pat.patch

messages: + msg162690
2012-06-08 00:14:28terry.reedysetmessages: + msg162511
2012-06-08 00:05:18python-devsetmessages: + msg162510
2012-06-05 21:06:52terry.reedysetfiles: + i12510c.test.diff

messages: + msg162373
2012-06-05 16:03:34roger.serwysetmessages: + msg162353
2012-06-05 09:55:43terry.reedysetmessages: + msg162336
2012-06-05 09:40:57terry.reedysetmessages: + msg162335
2012-06-05 09:21:08terry.reedysetfiles: + i12510b.test.diff

messages: + msg162334
2012-06-05 03:09:20roger.serwysetmessages: + msg162324
2012-06-04 03:01:53terry.reedysetfiles: + i12510.test.diff

messages: + msg162248
stage: needs patch -> patch review
2012-06-03 21:27:22roger.serwysetfiles: + fix_12510.patch

messages: + msg162233
2012-06-03 05:17:14terry.reedysetresolution: fixed -> (no value)
title: IDLE get_the_calltip mishandles raw strings -> IDLE: calltips mishandle raw strings and other examples
messages: + msg162195
stage: resolved -> needs patch
2012-06-03 05:07:31python-devsetmessages: + msg162192
2012-05-29 03:36:10roger.serwysetmessages: + msg161847
2012-05-29 01:43:40terry.reedysetstatus: closed -> open

messages: + msg161828
2012-05-28 03:14:40terry.reedysetassignee: terry.reedy
2012-05-28 01:55:03python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg161748

resolution: fixed
stage: commit review -> resolved
2012-05-28 00:20:54roger.serwysetmessages: + msg161743
2012-05-28 00:10:58terry.reedysetstage: commit review
2012-05-28 00:10:44terry.reedysetmessages: + msg161742
2012-05-27 20:19:23roger.serwysetfiles: + issue12510_rev1.patch

messages: + msg161721
2012-05-27 07:31:12terry.reedysetmessages: + msg161698
versions: + Python 2.7, Python 3.3
2011-12-21 18:37:53roger.serwysetfiles: + issue12510.patch

nosy: + roger.serwy
messages: + msg150033

keywords: + patch
2011-07-09 09:31:41ned.deilysetnosy: + ned.deily
messages: + msg140056
2011-07-09 05:42:15terry.reedysetnosy: + terry.reedy
messages: + msg140051
2011-07-06 23:27:21Roy.Foxcreate