classification
Title: import fixer misses some symbols
Type: behavior Stage:
Components: 2to3 (2.x to 3.x conversion tool) Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, collinwinter, mhammond, nedds
Priority: high Keywords: patch

Created on 2008-09-29 07:13 by mhammond, last changed 2008-11-26 04:03 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
fix_imports_bug.diff nedds, 2008-10-09 20:29 Fix_imports change and test case
Messages (5)
msg74014 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2008-09-29 07:13
The following source file:
"""
import _winreg
_winreg.OpenKey(_winreg.HKEY_LOCAL_MACHINE, "foo")
"""

results in the following "patch":

-import _winreg
-_winreg.OpenKey(_winreg.HKEY_LOCAL_MACHINE, "foo")
+import winreg
+winreg.OpenKey(_winreg.HKEY_LOCAL_MACHINE, "foo")

Note the first param has not been converted correctly.  This is probably
due to it following the paren, as using the same symbol in a local
variable works correctly.
msg74509 - (view) Author: Nick Edds (nedds) Date: 2008-10-08 01:54
I'll look in to it. Just give me a couple of days.
msg74550 - (view) Author: Nick Edds (nedds) Date: 2008-10-08 22:18
The problem is that fix_imports doesn't look at matches that are nested
within matches. So the _winreg.OpenKey part gets fixed, but the
_winreg.HKEY_LOCAL_MACHINE does not because it is nested within the
other node. I didn't make fix_imports so I don't know what the intention
was for not matching nested matches. When I removed that restriction,
the fixer works as expected. It also does not cause any of 2to3's tests
to fail nor does it have a noticeable impact on 2to3's runtime, so I'm
tempted to say that this is the fix to make, but I don't want to commit
to it until I've heard from it's author about it. I added Collin to the
Nosy List, so hopefully he can comment on the reasoning behind the
restriction.
msg74600 - (view) Author: Nick Edds (nedds) Date: 2008-10-09 20:29
Here is a patch with a new test case. I don't know if we should apply it
until we've heard from Collin though. The old version of fix_imports
fails the new test, but this version, which allows matching of nodes
nested within matches, passes the new test I added for nesting, as well
as all of the other 2to3 tests.
msg76446 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-26 04:03
I do have performance problems if I remove the match() override, but I
found that special casing usage replacements fixes the bug while keeping
speed.

Done in r67390. Nick, thanks for the debugging and the patch!
History
Date User Action Args
2008-11-26 04:03:58benjamin.petersonsetstatus: open -> closed
nosy: + benjamin.peterson
resolution: fixed
messages: + msg76446
2008-10-09 20:29:47neddssetfiles: + fix_imports_bug.diff
keywords: + patch
messages: + msg74600
2008-10-08 22:18:00neddssetnosy: + collinwinter
messages: + msg74550
2008-10-08 01:54:37neddssetmessages: + msg74509
2008-10-07 21:18:09benjamin.petersonsetnosy: + nedds
2008-09-29 07:13:11mhammondcreate