Patchwork [5,of,6] revset: parse alias declaration strictly by _parsealiasdecl

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Jan. 8, 2015, 10:37 a.m.
Message ID <f6735b97843b1d24bfff.1420713433@feefifofum>
Download mbox | patch
Permalink /patch/7385/
State Superseded
Commit aac4a1a7920e3cd7b1810f3d067ea6fa30aca9da
Headers show

Comments

Katsunori FUJIWARA - Jan. 8, 2015, 10:37 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1420712143 -32400
#      Thu Jan 08 19:15:43 2015 +0900
# Node ID f6735b97843b1d24bfff33f0e2636b2a9233551a
# Parent  9d7f9c153a0af1eb9ea201e9ca784f50670b3676
revset: parse alias declaration strictly by _parsealiasdecl

Before this patch, alias declaration is parsed by string base
operations: matching against "^([^(]+)\(([^)]+)\)$" and splitting by
",".

This overlooks many syntax errors like below (see the previous patch
introducing "_parsealiasdecl" for detail):

  - un-closed parenthesis causes being treated as "alias symbol"
  - symbol/function name aren't examined whether they are valid or not
  - invalid argument list causes unexpected argument names

To parse alias declaration strictly, this patch replaces parsing
implementation by "_parsealiasdecl".

This patch tests only one typical declaration error case, because
error detection itself is already tested in the doctest of
"_parsealiasdecl".

This also removes class property "args" and "error", because these are
certainly initialized in "revsetalias.__init__".

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2175,11 +2175,6 @@ 
         return (decl, None, None, parseerrordetail(inst))
 
 class revsetalias(object):
-    funcre = re.compile('^([^(]+)\(([^)]+)\)$')
-    args = None
-
-    # error message at parsing, or None
-    error = None
     # whether own `error` information is already shown or not.
     # this avoids showing same warning multiple times at each `findaliases`.
     warned = False
@@ -2190,18 +2185,17 @@ 
         h = heads(default)
         b($1) = ancestors($1) - ancestors(default)
         '''
-        m = self.funcre.search(name)
-        if m:
-            self.name = m.group(1)
-            self.tree = ('func', ('symbol', m.group(1)))
-            self.args = [x.strip() for x in m.group(2).split(',')]
+        self.name, self.tree, self.args, self.error = _parsealiasdecl(name)
+        if self.error:
+            self.error = _('failed to parse the declaration of revset alias'
+                           ' "%s": %s') % (self.name, self.error)
+            return
+
+        if self.args:
             for arg in self.args:
                 # _aliasarg() is an unknown symbol only used separate
                 # alias argument placeholders from regular strings.
                 value = value.replace(arg, '_aliasarg(%r)' % (arg,))
-        else:
-            self.name = name
-            self.tree = ('symbol', name)
 
         try:
             self.replacement, pos = parse(value)
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -969,6 +969,12 @@ 
   $ try 'tip'
   ('symbol', 'tip')
   9
+
+  $ hg debugrevspec --debug --config revsetalias.'bad name'='tip' "tip"
+  ('symbol', 'tip')
+  warning: failed to parse the declaration of revset alias "bad name": at 4: invalid token
+  9
+
   $ try 'd(2:5)'
   (func
     ('symbol', 'd')