From patchwork Wed Nov 25 06:31:40 2015
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [3,of,3] RFC check-code: try to detect @command/def errors
From: timeless@mozdev.org
X-Patchwork-Id: 11650
Message-Id: <c572fb376061421d6fec.1448433100@waste.org>
To: mercurial-devel@selenic.com
Date: Wed, 25 Nov 2015 00:31:40 -0600

# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1448432398 0
#      Wed Nov 25 06:19:58 2015 +0000
# Node ID c572fb376061421d6fec149a5876b155607d04ad
# Parent  a6abc5a4a6f757cca55f6db68520f934e0fa4057
RFC check-code: try to detect @command/def errors

This would have caught the bugs that were fixed in:
3836a70f1fa5
0fb08fcd5af8

It's admittedly a bit heavy-handed, but...
I'd also like to reduce the number of whitelisted warnings in
the .t -- just as I did by offering a patch for gpg

diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -84,6 +84,40 @@
     t = re.sub(r"\S", "x", m.group(2))
     return m.group(1) + t
 
+repcommandstr = '^@command\\(.([^,)]+).[,)]|^def ([^(]*)\('
+repcommandre = re.compile(repcommandstr)
+cmdbitsre = re.compile(r'\^?([^|]+)')
+
+def repcommandchecker(l, state):
+    if not 'cmd' in state:
+        state['commands'] = {}
+        state['defs'] = {}
+        state['cmd'] = None
+    match = repcommandre.search(l)
+    cmdname, defname = match.group(1), match.group(2)
+    result = []
+    if cmdname:
+        state['cmd'] = cmdname
+        cmdname = cmdbitsre.search(cmdname).group(1)
+        if cmdname in state['commands']:
+            result.append("duplicate @command: %s" % cmdname)
+        state['commands'][cmdname] = True
+    elif defname:
+        if defname in state['defs']:
+            result.append("duplicate def: %s" % defname)
+        cmd = state['cmd']
+        if cmd:
+            cmd_ = re.sub('[^a-z]', '', cmd)
+            defname_ = re.sub('[^a-z]', '', defname)
+            if not (re.search('%s(?:|_|cmd)' % cmd_, defname_) or
+                    re.search(defname_, cmd_)):
+                result.append("@command %s does not match def %s" %
+                    (cmd, defname))
+        state['cmd'] = ''
+        state['defs'][defname] = True
+    if len(result) == 0:
+        return None
+    return result
 
 testpats = [
   [
@@ -299,6 +333,11 @@
   # warnings
   [
     (r'(^| )pp +xxxxqq[ \n][^\n]', "add two newlines after '.. note::'"),
+    (repcommandstr,
+     "@command function should be unique and match function definition",
+     None,
+     repcommandchecker
+    ),
   ]
 ]
 
@@ -494,16 +533,21 @@
         prelines = None
         errors = []
         for i, pat in enumerate(pats):
-            if len(pat) == 3:
+            if len(pat) == 4:
+                p, msg, ignore, callback = pat
+            elif len(pat) == 3:
                 p, msg, ignore = pat
+                callback = None
             else:
                 p, msg = pat
                 ignore = None
+                callback = None
             if i >= nerrs:
                 msg = "warning: " + msg
 
             pos = 0
             n = 0
+            state = {}
             for m in p.finditer(post):
                 if prelines is None:
                     prelines = pre.splitlines()
@@ -523,6 +567,17 @@
                         print "Skipping %s for %s:%s (ignore pattern)" % (
                             name, f, n)
                     continue
+
+                if callback:
+                    result = callback(l, state)
+                    if not result:
+                        if debug:
+                            print "Skipping %s for %s:%s (bad match)" % (
+                                name, f, n)
+                        continue
+                    if debug:
+                        for e in result:
+                            errors.append((f, lineno and n + 1, l, e, ""))
                 bd = ""
                 if blame:
                     bd = 'working directory'
diff --git a/tests/test-check-code-hg.t b/tests/test-check-code-hg.t
--- a/tests/test-check-code-hg.t
+++ b/tests/test-check-code-hg.t
@@ -8,8 +8,15 @@
 
   $ hg locate | sed 's-\\-/-g' |
   >   xargs "$check_code" $HGTEST_DEBUG --warnings --per-file=0 || false
+  hgext/graphlog.py:52:
+   > def graphlog(ui, repo, *pats, **opts):
+   warning: @command function should be unique and match function definition
+  hgext/patchbomb.py:406:
+   > def patchbomb(ui, repo, *revs, **opts):
+   warning: @command function should be unique and match function definition
   Skipping hgext/zeroconf/Zeroconf.py it has no-che?k-code (glob)
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/socketutil.py it has no-che?k-code (glob)
+  [1]