[PATCH 3 of 3] RFC check-code: try to detect @command/def errors

[Yuya Nishihara]

On Wed, 25 Nov 2015 00:31:40 -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless at 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

Good idea.

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

It seems these hashes are wrong.

> 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

[...]

> +                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)

Because this isn't a style checking, maybe we can use the AST instead of
regexp black magic. I don't think new callback and state dict would be ideal
interface for this kind of checks.

Regards,