Patchwork revset: check invalid function syntax "func-name"() explicitly

login
register
mail settings
Submitter Yuya Nishihara
Date June 28, 2016, 2:31 p.m.
Message ID <8c584adb3a6357703faa.1467124292@mimosa>
Download mbox | patch
Permalink /patch/15652/
State Accepted
Headers show

Comments

Yuya Nishihara - June 28, 2016, 2:31 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1467027854 -32400
#      Mon Jun 27 20:44:14 2016 +0900
# Node ID 8c584adb3a6357703faa19634c807b181a8bbf5e
# Parent  6092992b98fa30d9ea30e0a05f4a7682b99862c6
revset: check invalid function syntax "func-name"() explicitly

Before the error was caught at func() as an unknown identifier, and the
optimizer failed to detect the syntax error. This patch introduces getsymbol()
helper to ensure that a string is not allowed as a function name.
Pierre-Yves David - June 30, 2016, 1:12 a.m.
On 06/28/2016 04:31 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1467027854 -32400
> #      Mon Jun 27 20:44:14 2016 +0900
> # Node ID 8c584adb3a6357703faa19634c807b181a8bbf5e
> # Parent  6092992b98fa30d9ea30e0a05f4a7682b99862c6
> revset: check invalid function syntax "func-name"() explicitly

Urg, subtle. Thanks for catching a fixing that. This patch is pushed.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -302,6 +302,11 @@  def tokenize(program, lookup=None, symin
 
 # helpers
 
+def getsymbol(x):
+    if x and x[0] == 'symbol':
+        return x[1]
+    raise error.ParseError(_('not a symbol'))
+
 def getstring(x, err):
     if x and (x[0] == 'string' or x[0] == 'symbol'):
         return x[1]
@@ -414,13 +419,14 @@  def keyvaluepair(repo, subset, k, v):
     raise error.ParseError(_("can't use a key-value pair in this context"))
 
 def func(repo, subset, a, b):
-    if a[0] == 'symbol' and a[1] in symbols:
-        return symbols[a[1]](repo, subset, b)
+    f = getsymbol(a)
+    if f in symbols:
+        return symbols[f](repo, subset, b)
 
     keep = lambda fn: getattr(fn, '__doc__', None) is not None
 
     syms = [s for (s, fn) in symbols.items() if keep(fn)]
-    raise error.UnknownIdentifier(a[1], syms)
+    raise error.UnknownIdentifier(f, syms)
 
 # functions
 
@@ -2304,11 +2310,11 @@  def _matchonly(revs, bases):
     """
     if (revs is not None
         and revs[0] == 'func'
-        and getstring(revs[1], _('not a symbol')) == 'ancestors'
+        and getsymbol(revs[1]) == 'ancestors'
         and bases is not None
         and bases[0] == 'not'
         and bases[1][0] == 'func'
-        and getstring(bases[1][1], _('not a symbol')) == 'ancestors'):
+        and getsymbol(bases[1][1]) == 'ancestors'):
         return ('list', revs[2], bases[1][2])
 
 def _optimize(x, small):
@@ -2419,7 +2425,7 @@  def _optimize(x, small):
         ws, ts = zip(*(_optimize(y, small) for y in x[1:]))
         return sum(ws), (op,) + ts
     elif op == 'func':
-        f = getstring(x[1], _("not a symbol"))
+        f = getsymbol(x[1])
         wa, ta = _optimize(x[2], small)
         if f in ("author branch closed date desc file grep keyword "
                  "outgoing user"):
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -434,6 +434,12 @@  quoting needed
   4
   $ hg book -d date
 
+function name should be a symbol
+
+  $ log '"date"(2005)'
+  hg: parse error: not a symbol
+  [255]
+
 keyword arguments
 
   $ log 'extra(branch, value=a)'
@@ -2033,6 +2039,16 @@  no crash by empty group "()" while optim
   hg: parse error: missing argument
   [255]
 
+invalid function call should not be optimized to only()
+
+  $ log '"ancestors"(6) and not ancestors(4)'
+  hg: parse error: not a symbol
+  [255]
+
+  $ log 'ancestors(6) and not "ancestors"(4)'
+  hg: parse error: not a symbol
+  [255]
+
 we can use patterns when searching for tags
 
   $ log 'tag("1..*")'