[Python-3000] Refactoring tool available (work in progress)

[Guido van Rossum]

On 12/20/06, Nick Coghlan <ncoghlan at gmail.com> wrote:
> Looking at a fix Georg just checked in for intern() - is there a way for the
> fixer to indicate to the refactoring tool that a particular module needs to be
> imported if the module being refactored triggers a match on the fix?

Not yet; I realize we need this, and I'm hoping for patches.

Inserting statements isn't as easy as you'd like using the current
state of the refactoring tool. The right place to pick is tricky: it
has to be after the docstring, and after any "from __future__ import
..." lines, but otherwise you'd like to put it as early as possible.
OTOH you might want to look for a row of familiar imports (os, re,
...) and insert or append it there. And of course it may already be
imported, in which case you don't need to do anything.

> In the intern() case, an entire module could be fixed just by putting "from
> sys import intern" at the start of the module, instead of fixing it at each
> invocation point.

Although in a sense that defeats the purpose. I'm not fond of
littering my code with "from M import F" statements because it doesn't
scale, namespace-wise. So I ould *rather* see the current solution
with the proper import statement added.

> This would also have the virtue of not breaking code that overrides intern() -
> because the new import would be at the start of the module, it would still be
> overridden, just like the current builtin.

Yes, this is a general problem -- I would like to add some kind of
namespace and scope analysis to the refactoring tool, so at least we
can tell where any particular name is defined (unless, of course, the
source is "from ... import *", in which case I respectfully give up
:-).

> For example, if a fix could provide a 'module_prefix' method which returned
> something to be inserted at the beginning of the module if the fix was
> triggered at least once, then the intern fix might look like:
>
> class FixIntern(object):
>      # No change to __init__ or match from Georg's checked in version
>
>      def module_prefix(self):
>          # Add 'from sys import intern' to the top of the module
>          return pytree.Node(syms.import_from,
>                            (pytree.Leaf(token.NAME, "sys"),
>                             pytree.Leaf(token.NAME, "intern")
>                            )) # Is that the right pytree incantation?

# Almost; remember it's a (very) concrete syntax tree, so you also
have to provide leaf nodes for the 'import' and 'from' keywords.

>      def transform(self, node):
>          # Actual occurences are left alone
>          return node

The driver currently ignores it if you return the original node
unchanged (or even an unchanged copy). But we can probably easily find
a slightly different way -- perhaps the driver calls a begin() method
at the start of a file, the transform() code can set a flag, and ad
the end of a file the driver will call an end() method which inserts
the new import if the flag was set. Or perhaps it should return an
import node and the driver should have some generic machinery that
inserts additional imports at the right place, using various
heuristics.

Care to run with this idea?

-- 
--Guido van Rossum (home page: http://www.python.org/~guido/)