[PATCH] util: eliminate wildcard imports

[Mads Kiilerich]

Matt Mackall wrote, On 07/23/2011 08:16 PM:
> On Sat, 2011-07-23 at 20:08 +0200, Adrian Buehlmann wrote:
>> On 2011-07-23 19:58, Matt Mackall wrote:
>>> On Sat, 2011-07-23 at 15:34 +0200, Adrian Buehlmann wrote:
>>>> # HG changeset patch
>>>> # User Adrian Buehlmann<adrian at cadifra.com>
>>>> # Date 1311416992 -7200
>>>> # Node ID f512f94029d77b9dd67455ae7c5668539f695edd
>>>> # Parent  ebdfdba0faafc5728b2dd4084fed50d06a6fad60
>>>> util: eliminate wildcard imports
>>> What's the motivation for this?
>> Mads :-)

Adrian first posted a patch that moved the wildcard import in util to 
the top of the file, with the noble goal of making util more readable so 
bugs like the one with util.localpath could be avoided. The first draft 
did however introduce a similar bug because it didn't consider that we 
relied on the precise location of the namespace pollution to implicitly 
override default implementations. I agree that it is good to have 
imports at the top of the file, but it only improves readability a 
little bit and I doubt it would have prevented the util.localpath collision.

This tells me that the problem with the wildcard import wasn't so much 
the location in the middle of the file but that it was so implicit and 
surprising what it really did. It _is_ hard to figure out exactly what 
is going on when 4 namespaces (including win32.py) are merged in two 
different ways - especially when we are used to the explicit and 
well-defined namespaces in Python. I think it would be less error-prone 
if it was more explicit what util.py really imports from windows.py and 
posix.py. I'm sure that would have prevented the util.localpath bug, and 
it would perhaps also have made it sufficiently explicit that it had 
consequences to move the imports.

> My observations are:
>
> - wildcard imports are ugly
> - ..but doing it manually is significantly uglier
> - wildcard imports defeats demand-loading
> - ..but so does doing it manually
> - wildcard imports are bad due to namespace pollution
> - ..but we actually want to pollute our namespace here

Yes, enumerating the imports explicitly and manually is ugly because it 
is so verbose and requires extra work when writing code. Wildcard 
imports are however ugly too because they make it so non-obvious what is 
imported and thus make it harder and more error-prone to grok, edit and 
refactor the code. That kind of ugliness is worse, IMO.

Namespace pollution with wildcard imports is ok as long as they have 
disjoint sets of definitions. It get more tricky if we have intentional 
or non-intentional conflicts - which will be resolved silently. The 
problem is how to check that we don't have or get unintended conflicts. 
My conclusion is that it is better to do the namespace pollution 
explicitly (if we want it all). That way a simple search within the .py 
will reveal any conflicts - as usual.

I think it would be an overall improvement to get rid of these last 
wildcard imports, even if the cost is some extra ugly verbosity.

(Duplicate 'from posix/windows import ...' lists would introduce some 
code duplication but it would be easy to maintain and thus perhaps be a 
slightly less ugly solution.)

/Mads