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

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Jan. 10, 2015, 2:25 p.m.
Message ID <950ed266b47516098bc3.1420899945@feefifofum>
Download mbox | patch
Permalink /patch/7424/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Jan. 10, 2015, 2:25 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1420899491 -32400
#      Sat Jan 10 23:18:11 2015 +0900
# Node ID 950ed266b47516098bc350b1b835c3de085c5e9f
# Parent  94f89514bcc23eb6f55b9919a46a35405cb5bc16
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
@@ -2197,11 +2197,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
@@ -2212,18 +2207,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
@@ -995,6 +995,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')