[PATCH 01 of 10] check-code: add AST check for mutable default argument values

Pierre-Yves David pierre-yves.david at ens-lyon.org
Wed Jan 4 15:14:01 UTC 2017



On 12/28/2016 06:43 PM, Augie Fackler wrote:
>
>> On Dec 27, 2016, at 4:57 AM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>
>>> This patch introduces a check-code check for the sanity of
>>> default argument values. It does so by parsing and walking the
>>> AST. Previously, all check-code checks were implemented as
>>> regular expressions. I probably could have implemented the check
>>> with regular expressions. However, AST based checks are more robust
>>> and arguably easier to implement for complex patterns (such as
>>> function argument lists).
>>
>> I'm a bit worried at check-code growing too big. There is probably existing python analysis tool to catch this kind of error. (eg: I'm prtty sure pylint have a warning for this). And I would rather leverage that work from other people rather than attempting to maintain our own full featured tool.
>>
>> Can you have a look in that other tools and tell us what you find?
>
> I know pylint can do this, but configuring pylint is a bit fussy.

I scanned quickly through the pylint help and got something up under two 
minutes. The small call below fits our needs regarding mutable default arg:

   pylint --disable=all --enable=W0102 --reports=no mercurial hgext …

I'll provide Greg with a .t version of that for his series.

Please try to avoid making unchecked negative statement about other 
people softwares.

> Any objection to landing the 2-10 patches without the check-code bit, and we can come back to pylint at a later date?

Yes, you can check my comment on the 2-10 patches for details.

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list