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
IDLE: calltips mishandle raw strings and other examples #56719
Comments
Hi, When you type (not copy-paste!) into an IDLE shell a string literal followed by ( 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:
Object: exec 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:
Object: exec 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 |
What platform? It sometimes makes a difference with tcl/tk and hence idle. |
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'." |
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. |
With 2.7 & 3.3 on Win 7, 'a'( brings up calltip (bad). 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.) |
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 bpo-14200 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. |
'a'(, [](, {}( no longer do anything. However, patch is not a complete fix yet. 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. |
I'm ok with removing the exception list, but add a comment like "# *any* exception could happen in the eval". |
New changeset 938b12452af7 by Terry Jan Reedy in branch '2.7': New changeset 4a7582866735 by Terry Jan Reedy in branch '3.2': New changeset 0835bee19f86 by Terry Jan Reedy in branch 'default': |
I am reopening just as a reminder to revise the comment. |
Which comment needs revision? |
New changeset 477508efe4ab by Terry Jan Reedy in branch '2.7': New changeset f927a5c6e4be by Terry Jan Reedy in branch '3.2': New changeset a7501ddf74ac by Terry Jan Reedy in branch 'default': |
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. |
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. |
i12510.test.diff (for Python 3 only) does the following:
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. |
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.
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. |
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 Adding the following to get_argspec:
& makes this new test pass. Revised patch with above attached. Note 1: Programming is more fun with decent tests. |
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.) |
Upon further thought, not a problem. Stephen considered the case: 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'. |
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.
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?
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__'):
if not argspec:
argspec += "Arguments unknown. See docs."
return argspec |
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 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. |
New changeset 02b4c62ce393 by Terry Jan Reedy in branch '3.2': New changeset 03b5f75ddac7 by Terry Jan Reedy in branch 'default': |
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. |
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. |
New changeset eea379307efa by Terry Jan Reedy in branch '3.2': New changeset 464c6a50b0ce by Terry Jan Reedy in branch 'default': |
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. |
Are there any other aspects of the calltip handler that requires fixing? If not, then I can start back-porting the patches. |
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 ;-). |
I haven't had any serious problems with calltips on the dev branches for 2.7 and 3.4, except for bpo-16630. 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. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: