[PATCH 03 of 10] context: don't use mutable default argument value

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Dec 27 05:03:03 EST 2016



On 12/27/2016 01:03 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1482796356 25200
> #      Mon Dec 26 16:52:36 2016 -0700
> # Node ID 10645505563a162bb6d91a90e8b3ded769dcdbb1
> # Parent  6d16de1b79686f5790764afeafb7cc2a4af0a1bb
> context: don't use mutable default argument value
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -289,10 +289,11 @@ class basectx(object):
>          context.
>          '''
>          return subrepo.subrepo(self, path, allowwdir=True)
>
> -    def match(self, pats=[], include=None, exclude=None, default='glob',
> +    def match(self, pats=None, include=None, exclude=None, default='glob',
>                listsubrepos=False, badfn=None):
> +        pats = pats or []


Same as before, we should explicitly test for 'None' here. Otherwise the 
code will behave differently (reuse the list vs create a new one) when 
passing an empty list as argument.

An alternative would be to use '()' if the code is readonly. That would 
prevent later mutability bug and acknowledge the parameters is readonly.

(I don't often advocate for mixing tuple and list but this is one of the 
rare case)

>          r = self._repo
>          return matchmod.match(r.root, r.getcwd(), pats,
>                                include, exclude, default,
>                                auditor=r.nofsauditor, ctx=self,
> @@ -1494,10 +1495,11 @@ class workingctx(committablectx):
>                  elif self._repo.dirstate[dest] in 'r':
>                      self._repo.dirstate.normallookup(dest)
>                  self._repo.dirstate.copy(source, dest)
>
> -    def match(self, pats=[], include=None, exclude=None, default='glob',
> +    def match(self, pats=None, include=None, exclude=None, default='glob',
>                listsubrepos=False, badfn=None):
> +        pats = pats or []
>          r = self._repo
>
>          # Only a case insensitive filesystem needs magic to translate user input
>          # to actual case in the filesystem.
> diff --git a/tests/test-check-code.t b/tests/test-check-code.t
> --- a/tests/test-check-code.t
> +++ b/tests/test-check-code.t
> @@ -39,14 +39,8 @@ New errors are not allowed. Warnings are
>    Skipping i18n/polib.py it has no-che?k-code (glob)
>    mercurial/changegroup.py:260:
>     >               targetphase=phases.draft, expectedtotal=None):
>     attribute default argument value may be mutable
> -  mercurial/context.py:293:
> -   >     def match(self, pats=[], include=None, exclude=None, default='glob',
> -   mutable default argument value (list)
> -  mercurial/context.py:1498:
> -   >     def match(self, pats=[], include=None, exclude=None, default='glob',
> -   mutable default argument value (list)
>    mercurial/demandimport.py:309:
>     >     if os.environ.get('HGDEMANDIMPORT') != 'disable':
>     use encoding.environ instead (py3)
>    mercurial/encoding.py:54:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list