Patchwork [v2] check-code: skip unhandled files

login
register
mail settings
Submitter timeless
Date May 11, 2016, 5:32 p.m.
Message ID <12720ea9628e81605c45.1462987921@gcc2-power8.osuosl.org>
Download mbox | patch
Permalink /patch/15026/
State Rejected
Delegated to: Yuya Nishihara
Headers show

Comments

timeless - May 11, 2016, 5:32 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1462930640 0
#      Wed May 11 01:37:20 2016 +0000
# Node ID 12720ea9628e81605c45c4c7a11e9b8a0902eccc
# Parent  c641b8dfb98c2ade6995ba3aa341fe4d7b154827
# Available At bb://timeless/mercurial-crew
#              hg pull bb://timeless/mercurial-crew -r 12720ea9628e
check-code: skip unhandled files

check-code is organized into a list of file patterns for which there
are a list of string patterns in which to look for strings (and then
complain if matches are found).

It doesn't make sense to open files for which there are no matching
file patterns. So, we compile the matching file patterns and then
skip anything that doesn't match them.
Yuya Nishihara - May 18, 2016, 12:02 p.m.
On Wed, 11 May 2016 17:32:01 +0000, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1462930640 0
> #      Wed May 11 01:37:20 2016 +0000
> # Node ID 12720ea9628e81605c45c4c7a11e9b8a0902eccc
> # Parent  c641b8dfb98c2ade6995ba3aa341fe4d7b154827
> # Available At bb://timeless/mercurial-crew
> #              hg pull bb://timeless/mercurial-crew -r 12720ea9628e
> check-code: skip unhandled files
> 
> check-code is organized into a list of file patterns for which there
> are a list of string patterns in which to look for strings (and then
> complain if matches are found).
> 
> It doesn't make sense to open files for which there are no matching
> file patterns. So, we compile the matching file patterns and then
> skip anything that doesn't match them.
> 
> diff -r c641b8dfb98c -r 12720ea9628e contrib/check-code.py
> --- a/contrib/check-code.py	Wed May 11 01:46:11 2016 +0000
> +++ b/contrib/check-code.py	Wed May 11 01:37:20 2016 +0000
> @@ -426,6 +426,8 @@
>      ('web template', r'mercurial/templates/.*\.tmpl', '',
>       webtemplatefilters, webtemplatepats),
>  ]
> +filechecks = r'|'.join([c[1] for c in checks])
> +filecheckre = re.compile(filechecks)
>  
>  def _preparepats():
>      for c in checks:
> @@ -615,6 +617,10 @@
>  
>      ret = 0
>      for f in check:
> +        if not filecheckre.search(f):
> +            if options.debug:
> +                print("Ignoring %s" % f)
> +            continue

This breaks check for Python files without .py suffix. It has to read at least
one line to test magic.
Martijn Pieters - May 18, 2016, 12:25 p.m.
On 11 May 2016 at 18:32, timeless <timeless@fmr.im> wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1462930640 0
> #      Wed May 11 01:37:20 2016 +0000
> # Node ID 12720ea9628e81605c45c4c7a11e9b8a0902eccc
> # Parent  c641b8dfb98c2ade6995ba3aa341fe4d7b154827
> # Available At bb://timeless/mercurial-crew
> #              hg pull bb://timeless/mercurial-crew -r 12720ea9628e
> check-code: skip unhandled files
>
> check-code is organized into a list of file patterns for which there
> are a list of string patterns in which to look for strings (and then
> complain if matches are found).
>
> It doesn't make sense to open files for which there are no matching
> file patterns. So, we compile the matching file patterns and then
> skip anything that doesn't match them.
>
> diff -r c641b8dfb98c -r 12720ea9628e contrib/check-code.py
> --- a/contrib/check-code.py     Wed May 11 01:46:11 2016 +0000
> +++ b/contrib/check-code.py     Wed May 11 01:37:20 2016 +0000
> @@ -426,6 +426,8 @@
>      ('web template', r'mercurial/templates/.*\.tmpl', '',
>       webtemplatefilters, webtemplatepats),
>  ]
> +filechecks = r'|'.join([c[1] for c in checks])

When joining regex patters I'd add `(...)` parentheses around each to
allow for further `|` pipes in each pattern.

> +filecheckre = re.compile(filechecks)
>
>  def _preparepats():
>      for c in checks:
> @@ -615,6 +617,10 @@
>
>      ret = 0
>      for f in check:
> +        if not filecheckre.search(f):
> +            if options.debug:
> +                print("Ignoring %s" % f)
> +            continue

You are ignoring the *magic* column in `checks` here; files are
checked either when their filename matches checks[N][1] **or** when
the string in checks[N][1] is not empty and found in the file
contents.

>          if not checkfile(f, maxerr=options.per_file, warnings=options.warnings,
>                           blame=options.blame, debug=options.debug,
>                           lineno=options.lineno):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff -r c641b8dfb98c -r 12720ea9628e contrib/check-code.py
--- a/contrib/check-code.py	Wed May 11 01:46:11 2016 +0000
+++ b/contrib/check-code.py	Wed May 11 01:37:20 2016 +0000
@@ -426,6 +426,8 @@ 
     ('web template', r'mercurial/templates/.*\.tmpl', '',
      webtemplatefilters, webtemplatepats),
 ]
+filechecks = r'|'.join([c[1] for c in checks])
+filecheckre = re.compile(filechecks)
 
 def _preparepats():
     for c in checks:
@@ -615,6 +617,10 @@ 
 
     ret = 0
     for f in check:
+        if not filecheckre.search(f):
+            if options.debug:
+                print("Ignoring %s" % f)
+            continue
         if not checkfile(f, maxerr=options.per_file, warnings=options.warnings,
                          blame=options.blame, debug=options.debug,
                          lineno=options.lineno):