[PATCH 2 of 3 RFC] Remove all uses of hasattr(a, b) in favor of getattr(a, b, None)

[Martin Geisler]

Dirkjan Ochtman <dirkjan at ochtman.nl> writes:

> On Thu, Mar 3, 2011 at 11:31, Martin Geisler <mg at lazybytes.net> wrote:
>>> # HG changeset patch
>>> # User Augie Fackler <durin42 at gmail.com>
>>> # Date 1299024193 21600
>>> # Branch stable
>>> # Node ID 75e012dfe383900f1932bdb0831eb44a11d35876
>>> # Parent  f202bb8bc5e81a41893144065cc634a786d549b0
>>> Remove all uses of hasattr(a, b) in favor of getattr(a, b, None)
>>>
>>> hasattr() in Python swallows all exceptions rather than only
>>> AttributeError, making it unsafe for general use. getattr(a, b, None)
>>> is a mostly-compatible replacement (it fails to distinguish missing
>>> attributes and None-value attributes) that we can use instead.
>>
>> Okay, I get that hasattr swallows all exceptions, but is that not only a
>> problem when you try to access an attribute of an unloaded module (due
>> to demandimport) and the import that triggers a exception other than
>> AttributeError?
>
> It gets problematic with any property() too, or __getattr__, etc.
>
> And if you want to enforce this through check-code, you really want to
> get rid of every mention, I think?

Yes, that is true, otherwise check-code wont work.

>>>              doc = doc()
>>>          ui.write(doc)
>>>          ui.write("\n")
>>
>>>      # Windows does not support GIT_DIR= construct while other systems
>>>      # cannot remove environment variable. Just assume none have
>>>      # both issues.
>>> -    if hasattr(os, 'unsetenv'):
>>> +    if getattr(os, 'unsetenv', None):
>>
>> Also here, the os module can hardly throw weird exceptions when it's
>> imported by demandimport?
>
> I'd say demandimport is actually an area where it helps if we don't
> swallow exceptions. Who knows what happens deep in the bowels of
> demandimport?

Well, it is fairly simple: hand back a proxy module when you ask for a
module, load the real module when you access attributes of the proxy
object for the first time, then forward all attribute access
transparently.

My concern is only that this makes the code look strange. But now that
we've discussed why the code looks strange I'm okay with the change.

-- 
Martin Geisler

aragost Trifork
Professional Mercurial support
http://aragost.com/en/services/mercurial/blog/