[Pull Request] Remove uses of hasattr

[Adrian Buehlmann]

On 2011-07-26 11:40, Adrian Buehlmann wrote:
> On 2011-07-26 05:44, Augie Fackler wrote:
>> A while ago I asked about removing hasattr because of surprising behavior, and the general consensus seemed to be that I should go ahead and make a run at it. While on a flight today I finally finished the job. It's a really long patch queue, so instead of mailing it I'm submitting it as a pull request. You can see the changesets here:
>>
>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/list?num=50
>>
>> and if you want to pull/incoming, the URL you want is
>>
>> http://code.google.com/p/durin42-hg-hasattr-ectomy/
>>
>> It's 38 patches in total. I tried to batch them for easy reviewing. Ones of note are:
>>
>> Duplicate less code when checking to see if we're in a frozen binary:
>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=b6581ed193f374f2d38a4cad5ad25640f304ce1f
>>
>> These two attack a particular hasattr() use case in the entire codebase since it's mechanically fixable:
>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=f4814ebb99f75812a55776ed3a306883432935dd
>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=235f15a351866c2c084d4e88b89f7b69fafa66da
>>
>> and this last one of note adds modules to the blacklist because of those side effects of hasattr() I warned about:
>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=57c2982d243bd6651e1d9e3f138f46b27b32828f
>>
>> Other than that, it's mostly mechanical replacement, with a few migrations to getattr() so we can avoid looking up the attribute twice. I tried to make the patches all tiny so they'd be easy to review.
> 
> Augie asked to pull:
>> # HG changeset patch
>> # User Augie Fackler <durin42 at gmail.com>
>> # Date 1311623995 18000
>> # Node ID 9b736a33999292cf7fe5a8e0edbd145c9dd32e19
>> # Parent  b6581ed193f374f2d38a4cad5ad25640f304ce1f
>> safehasattr: new function to work around hasattr being broken
>>
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -29,6 +29,10 @@
>>  def sha1(s):
>>      return _fastsha1(s)
>>  
>> +_notset = object()
>> +def safehasattr(thing, attr):
>> +    return getattr(thing, attr, _notset) is not _notset
>> +
>>  def _fastsha1(s):
>>      # This function will import sha1 from hashlib or sha (whichever is
>>      # available) and overwrite itself with it on the first call.
> 
> IMHO, it would have been nice to write in the change message in what
> ways hasattr is broken.
> 
> Or at least put a pointer to an explanation.

Augie patched:
> # HG changeset patch
> # User Augie Fackler <durin42 at gmail.com>
> # Date 1311623971 18000
> # Node ID e7859521b23b358659ab8a918677ccaf3ed46008
> # Parent  27f3169fd8f4affa374201af287698d5f59c920f
> check-code: disallow use of hasattr()

Or perhaps the explanation why hasattr is bad could be given in this
change message (it's the topmost patch)