check-code.py
875 lines
| 32.2 KiB
| text/x-python
|
PythonLexer
/ contrib / check-code.py
|
r10281 | #!/usr/bin/env python | ||
# | ||||
# check-code - a style and portability checker for Mercurial | ||||
# | ||||
|
r10290 | # Copyright 2010 Matt Mackall <mpm@selenic.com> | ||
|
r10281 | # | ||
# This software may be used and distributed according to the terms of the | ||||
# GNU General Public License version 2 or any later version. | ||||
|
r20241 | """style and portability checker for Mercurial | ||
when a rule triggers wrong, do one of the following (prefer one from top): | ||||
* do the work-around the rule suggests | ||||
* doublecheck that it is a false match | ||||
* improve the rule pattern | ||||
* add an ignore pattern to the rule (3rd arg) which matches your good line | ||||
|
r28700 | (you can append a short comment and match this, like: #re-raises) | ||
|
r20241 | * change the pattern to a warning and list the exception in test-check-code-hg | ||
* ONLY use no--check-code for skipping entire files from external sources | ||||
""" | ||||
|
r28509 | from __future__ import absolute_import, print_function | ||
import glob | ||||
|
r13074 | import keyword | ||
|
r10895 | import optparse | ||
|
r28509 | import os | ||
import re | ||||
import sys | ||||
|
r29145 | if sys.version_info[0] < 3: | ||
opentext = open | ||||
else: | ||||
def opentext(f): | ||||
|
r39091 | return open(f, encoding='latin1') | ||
|
r19310 | try: | ||
|
r29143 | xrange | ||
except NameError: | ||||
xrange = range | ||||
try: | ||||
|
r19310 | import re2 | ||
except ImportError: | ||||
re2 = None | ||||
|
r41992 | import testparseutil | ||
|
r19310 | def compilere(pat, multiline=False): | ||
if multiline: | ||||
pat = '(?m)' + pat | ||||
if re2: | ||||
try: | ||||
return re2.compile(pat) | ||||
except re2.error: | ||||
pass | ||||
return re.compile(pat) | ||||
|
r10281 | |||
|
r29398 | # check "rules depending on implementation of repquote()" in each | ||
# patterns (especially pypats), before changing around repquote() | ||||
_repquotefixedmap = {' ': ' ', '\n': '\n', '.': 'p', ':': 'q', | ||||
'%': '%', '\\': 'b', '*': 'A', '+': 'P', '-': 'M'} | ||||
def _repquoteencodechr(i): | ||||
if i > 255: | ||||
return 'u' | ||||
c = chr(i) | ||||
if c in _repquotefixedmap: | ||||
return _repquotefixedmap[c] | ||||
if c.isalpha(): | ||||
return 'x' | ||||
if c.isdigit(): | ||||
return 'n' | ||||
return 'o' | ||||
_repquotett = ''.join(_repquoteencodechr(i) for i in xrange(256)) | ||||
|
r10281 | def repquote(m): | ||
|
r19999 | t = m.group('text') | ||
|
r29398 | t = t.translate(_repquotett) | ||
|
r10722 | return m.group('quote') + t + m.group('quote') | ||
|
r10281 | |||
|
r10727 | def reppython(m): | ||
comment = m.group('comment') | ||||
if comment: | ||||
|
r18959 | l = len(comment.rstrip()) | ||
return "#" * l + comment[l:] | ||||
|
r10727 | return repquote(m) | ||
|
r10281 | |||
def repcomment(m): | ||||
return m.group(1) + "#" * len(m.group(2)) | ||||
def repccomment(m): | ||||
t = re.sub(r"((?<=\n) )|\S", "x", m.group(2)) | ||||
return m.group(1) + t + "*/" | ||||
def repcallspaces(m): | ||||
t = re.sub(r"\n\s+", "\n", m.group(2)) | ||||
return m.group(1) + t | ||||
def repinclude(m): | ||||
return m.group(1) + "<foo>" | ||||
def rephere(m): | ||||
t = re.sub(r"\S", "x", m.group(2)) | ||||
return m.group(1) + t | ||||
testpats = [ | ||||
|
r14009 | [ | ||
|
r31877 | (r'\b(push|pop)d\b', "don't use 'pushd' or 'popd', use 'cd'"), | ||
|
r15281 | (r'\W\$?\(\([^\)\n]*\)\)', "don't use (()) or $(()), use 'expr'"), | ||
|
r10374 | (r'grep.*-q', "don't use 'grep -q', redirect to /dev/null"), | ||
|
r27989 | (r'(?<!hg )grep.* -a', "don't use 'grep -a', use in-line python"), | ||
|
r16332 | (r'sed.*-i', "don't use 'sed -i', use a temporary file"), | ||
|
r16965 | (r'\becho\b.*\\n', "don't use 'echo \\n', use printf"), | ||
|
r11884 | (r'echo -n', "don't use 'echo -n', use printf"), | ||
|
r23134 | (r'(^|\|\s*)\bwc\b[^|]*$\n(?!.*\(re\))', "filter wc output"), | ||
|
r10374 | (r'head -c', "don't use 'head -c', use 'dd'"), | ||
|
r19628 | (r'tail -n', "don't use the '-n' option to tail, just use '-<num>'"), | ||
|
r15389 | (r'sha1sum', "don't use sha1sum, use $TESTDIR/md5sum.py"), | ||
|
r37258 | (r'\bls\b.*-\w*R', "don't use 'ls -R', use 'find'"), | ||
|
r29142 | (r'printf.*[^\\]\\([1-9]|0\d)', r"don't use 'printf \NNN', use Python"), | ||
|
r19380 | (r'printf.*[^\\]\\x', "don't use printf \\x, use Python"), | ||
|
r10281 | (r'rm -rf \*', "don't use naked rm -rf, target a directory"), | ||
|
r32293 | (r'\[[^\]]+==', '[ foo == bar ] is a bashism, use [ foo = bar ] instead'), | ||
|
r15372 | (r'(^|\|\s*)grep (-\w\s+)*[^|]*[(|]\w', | ||
|
r10281 | "use egrep for extended grep syntax"), | ||
|
r34063 | (r'(^|\|\s*)e?grep .*\\S', "don't use \\S in regular expression"), | ||
|
r34062 | (r'(?<!!)/bin/', "don't use explicit paths for tools"), | ||
(r'#!.*/bash', "don't use bash in shebang, use sh"), | ||||
|
r10281 | (r'[^\n]\Z', "no trailing newline"), | ||
|
r27791 | (r'export .*=', "don't export and assign at once"), | ||
|
r15372 | (r'^source\b', "don't use 'source', use '.'"), | ||
|
r12367 | (r'touch -d', "don't use 'touch -d', use 'touch -t' instead"), | ||
|
r29330 | (r'\bls +[^|\n-]+ +-', "options to 'ls' must come before filenames"), | ||
|
r15281 | (r'[^>\n]>\s*\$HGRCPATH', "don't overwrite $HGRCPATH, append to it"), | ||
|
r15372 | (r'^stop\(\)', "don't use 'stop' as a shell function name"), | ||
|
r15282 | (r'(\[|\btest\b).*-e ', "don't use 'test -e', use 'test -f'"), | ||
|
r25588 | (r'\[\[\s+[^\]]*\]\]', "don't use '[[ ]]', use '[ ]'"), | ||
|
r16013 | (r'^alias\b.*=', "don't use alias, use a function"), | ||
|
r16485 | (r'if\s*!', "don't use '!' to negate exit status"), | ||
|
r16494 | (r'/dev/u?random', "don't use entropy, use /dev/zero"), | ||
|
r16496 | (r'do\s*true;\s*done', "don't use true as loop body, use sleep 0"), | ||
|
r19083 | (r'sed (-e )?\'(\d+|/[^/]*/)i(?!\\\n)', | ||
|
r19080 | "put a backslash-escaped newline after sed 'i' command"), | ||
|
r27557 | (r'^diff *-\w*[uU].*$\n(^ \$ |^$)', "prefix diff -u/-U with cmp"), | ||
(r'^\s+(if)? diff *-\w*[uU]', "prefix diff -u/-U with cmp"), | ||||
|
r33288 | (r'[\s="`\']python\s(?!bindings)', "don't use 'python', use '$PYTHON'"), | ||
|
r26588 | (r'seq ', "don't use 'seq', use $TESTDIR/seq.py"), | ||
(r'\butil\.Abort\b', "directly use error.Abort"), | ||||
|
r26777 | (r'\|&', "don't use |&, use 2>&1"), | ||
|
r27640 | (r'\w = +\w', "only one space after = allowed"), | ||
|
r28781 | (r'\bsed\b.*[^\\]\\n', "don't use 'sed ... \\n', use a \\ and a newline"), | ||
|
r30557 | (r'env.*-u', "don't use 'env -u VAR', use 'unset VAR'"), | ||
(r'cp.* -r ', "don't use 'cp -r', use 'cp -R'"), | ||||
r35086 | (r'grep.* -[ABC]', "don't use grep's context flags"), | |||
|
r35252 | (r'find.*-printf', | ||
"don't use 'find -printf', it doesn't exist on BSD find(1)"), | ||||
|
r36182 | (r'\$RANDOM ', "don't use bash-only $RANDOM to generate random values"), | ||
|
r14009 | ], | ||
# warnings | ||||
|
r16672 | [ | ||
(r'^function', "don't use 'function', use old style"), | ||||
(r'^diff.*-\w*N', "don't use 'diff -N'"), | ||||
|
r18508 | (r'\$PWD|\${PWD}', "don't use $PWD, use `pwd`"), | ||
|
r16672 | (r'^([^"\'\n]|("[^"\n]*")|(\'[^\'\n]*\'))*\^', "^ must be quoted"), | ||
|
r18575 | (r'kill (`|\$\()', "don't use kill, use killdaemons.py") | ||
|
r16672 | ] | ||
|
r10281 | ] | ||
testfilters = [ | ||||
|
r34062 | (r"( *)(#([^!][^\n]*\S)?)", repcomment), | ||
|
r10281 | (r"<<(\S+)((.|\n)*?\n\1)", rephere), | ||
] | ||||
|
r15372 | uprefix = r"^ \$ " | ||
|
r12364 | utestpats = [ | ||
|
r14009 | [ | ||
|
r27693 | (r'^(\S.*|| [$>] \S.*)[ \t]\n', "trailing whitespace on non-output"), | ||
|
r16673 | (uprefix + r'.*\|\s*sed[^|>\n]*\n', | ||
"use regex test output patterns instead of sed"), | ||||
|
r12364 | (uprefix + r'(true|exit 0)', "explicit zero exit unnecessary"), | ||
|
r15607 | (uprefix + r'.*(?<!\[)\$\?', "explicit exit code checks unnecessary"), | ||
|
r12364 | (uprefix + r'.*\|\| echo.*(fail|error)', | ||
"explicit exit code checks unnecessary"), | ||||
(uprefix + r'set -e', "don't use set -e"), | ||||
|
r19873 | (uprefix + r'(\s|fi\b|done\b)', "use > for continued lines"), | ||
|
r20423 | (uprefix + r'.*:\.\S*/', "x:.y in a path does not work on msys, rewrite " | ||
"as x://.y, or see `hg log -k msys` for alternatives", r'-\S+:\.|' #-Rxxx | ||||
|
r24205 | '# no-msys'), # in test-pull.t which is skipped on windows | ||
|
r31816 | (r'^ [^$>].*27\.0\.0\.1', | ||
'use $LOCALIP not an explicit loopback address'), | ||||
|
r35154 | (r'^ (?![>$] ).*\$LOCALIP.*[^)]$', | ||
|
r31816 | 'mark $LOCALIP output lines with (glob) to help tests in BSD jails'), | ||
|
r35463 | (r'^ (cat|find): .*: \$ENOENT\$', | ||
|
r21930 | 'use test -f to test for file existence'), | ||
|
r28033 | (r'^ diff -[^ -]*p', | ||
"don't use (external) diff with -p for portability"), | ||||
|
r34574 | (r' readlink ', 'use readlink.py instead of readlink'), | ||
|
r28034 | (r'^ [-+][-+][-+] .* [-+]0000 \(glob\)', | ||
"glob timezone field in diff output for portability"), | ||||
|
r28035 | (r'^ @@ -[0-9]+ [+][0-9]+,[0-9]+ @@', | ||
"use '@@ -N* +N,n @@ (glob)' style chunk header for portability"), | ||||
(r'^ @@ -[0-9]+,[0-9]+ [+][0-9]+ @@', | ||||
"use '@@ -N,n +N* @@ (glob)' style chunk header for portability"), | ||||
(r'^ @@ -[0-9]+ [+][0-9]+ @@', | ||||
"use '@@ -N* +N* @@ (glob)' style chunk header for portability"), | ||||
|
r28053 | (uprefix + r'hg( +-[^ ]+( +[^ ]+)?)* +extdiff' | ||
r'( +(-[^ po-]+|--(?!program|option)[^ ]+|[^-][^ ]*))*$', | ||||
"use $RUNTESTDIR/pdiff via extdiff (or -o/-p for false-positives)"), | ||||
|
r14009 | ], | ||
# warnings | ||||
|
r18683 | [ | ||
|
r31673 | (r'^ (?!.*\$LOCALIP)[^*?/\n]* \(glob\)$', | ||
"glob match with no glob string (?, *, /, and $LOCALIP)"), | ||||
|
r18683 | ] | ||
|
r12364 | ] | ||
|
r35316 | # transform plain test rules to unified test's | ||
|
r14203 | for i in [0, 1]: | ||
|
r22101 | for tp in testpats[i]: | ||
p = tp[0] | ||||
m = tp[1] | ||||
|
r15372 | if p.startswith(r'^'): | ||
|
r16672 | p = r"^ [$>] (%s)" % p[1:] | ||
|
r14203 | else: | ||
|
r16672 | p = r"^ [$>] .*(%s)" % p | ||
|
r22101 | utestpats[i].append((p, m) + tp[2:]) | ||
|
r12364 | |||
|
r35316 | # don't transform the following rules: | ||
# " > \t" and " \t" should be allowed in unified tests | ||||
testpats[0].append((r'^( *)\t', "don't use tabs to indent")) | ||||
utestpats[0].append((r'^( ?)\t', "don't use tabs to indent")) | ||||
|
r12364 | utestfilters = [ | ||
|
r17711 | (r"<<(\S+)((.|\n)*?\n > \1)", rephere), | ||
|
r34062 | (r"( +)(#([^!][^\n]*\S)?)", repcomment), | ||
|
r12364 | ] | ||
|
r41987 | # common patterns to check *.py | ||
commonpypats = [ | ||||
|
r14009 | [ | ||
|
r41927 | (r'\\$', 'Use () to wrap long lines in Python, not \\'), | ||
|
r11568 | (r'^\s*def\s*\w+\s*\(.*,\s*\(', | ||
"tuple parameter unpacking not available in Python 3+"), | ||||
(r'lambda\s*\(.*,.*\)', | ||||
"tuple parameter unpacking not available in Python 3+"), | ||||
|
r11764 | (r'(?<!def)\s+(cmp)\(', "cmp is not available in Python 3+"), | ||
|
r30883 | (r'(?<!\.)\breduce\s*\(.*', "reduce is not available in Python 3+"), | ||
|
r29793 | (r'\bdict\(.*=', 'dict() is different in Py2 and 3 and is slower than {}', | ||
|
r20688 | 'dict-from-generator'), | ||
|
r11602 | (r'\.has_key\b', "dict.has_key is not available in Python 3+"), | ||
|
r18183 | (r'\s<>\s', '<> operator is not available in Python 3+, use !='), | ||
|
r10281 | (r'^\s*\t', "don't use tabs"), | ||
|
r10412 | (r'\S;\s*\n', "semicolon"), | ||
|
r21097 | (r'[^_]_\([ \t\n]*(?:"[^"]+"[ \t\n+]*)+%', "don't use % inside _()"), | ||
(r"[^_]_\([ \t\n]*(?:'[^']+'[ \t\n+]*)+%", "don't use % inside _()"), | ||||
|
r18054 | (r'(\w|\)),\w', "missing whitespace after ,"), | ||
(r'(\w|\))[+/*\-<>]\w', "missing whitespace in expression"), | ||||
|
r18055 | (r'^\s+(\w|\.)+=\w[^,()\n]*$', "missing whitespace in assignment"), | ||
|
r27640 | (r'\w\s=\s\s+\w', "gratuitous whitespace after ="), | ||
|
r34383 | (( | ||
# a line ending with a colon, potentially with trailing comments | ||||
r':([ \t]*#[^\n]*)?\n' | ||||
# one that is not a pass and not only a comment | ||||
r'(?P<indent>[ \t]+)[^#][^\n]+\n' | ||||
# more lines at the same indent level | ||||
r'((?P=indent)[^\n]+\n)*' | ||||
# a pass at the same indent level, which is bogus | ||||
r'(?P=indent)pass[ \t\n#]' | ||||
), 'omit superfluous pass'), | ||||
|
r10281 | (r'[^\n]\Z', "no trailing newline"), | ||
|
r15281 | (r'(\S[ \t]+|^[ \t]+)\n', "trailing whitespace"), | ||
|
r16683 | # (r'^\s+[^_ \n][^_. \n]+_[^_\n]+\s*=', | ||
# "don't use underbars in identifiers"), | ||||
|
r34084 | (r'^\s+(self\.)?[A-Za-z][a-z0-9]+[A-Z]\w* = ', | ||
|
r34430 | "don't use camelcase in identifiers", r'#.*camelcase-required'), | ||
|
r15281 | (r'^\s*(if|while|def|class|except|try)\s[^[\n]*:\s*[^\\n]#\s]+', | ||
|
r10286 | "linebreak after :"), | ||
|
r28219 | (r'class\s[^( \n]+:', "old-style class, use class foo(object)", | ||
r'#.*old-style'), | ||||
|
r15281 | (r'class\s[^( \n]+\(\):', | ||
|
r28219 | "class foo() creates old style object, use class foo(object)", | ||
r'#.*old-style'), | ||||
|
r25028 | (r'\b(%s)\(' % '|'.join(k for k in keyword.kwlist | ||
if k not in ('print', 'exec')), | ||||
|
r13076 | "Python keyword is not a function"), | ||
|
r10412 | (r',]', "unneeded trailing ',' in list"), | ||
|
r10281 | # (r'class\s[A-Z][^\(]*\((?!Exception)', | ||
# "don't capitalize non-exception classes"), | ||||
# (r'in range\(', "use xrange"), | ||||
# (r'^\s*print\s+', "avoid using print in core and extensions"), | ||||
(r'[\x80-\xff]', "non-ASCII character literal"), | ||||
|
r25212 | (r'("\')\.format\(', "str.format() has no bytes counterpart, use %"), | ||
|
r13074 | (r'^\s*(%s)\s\s' % '|'.join(keyword.kwlist), | ||
"gratuitous whitespace after Python keyword"), | ||||
|
r15281 | (r'([\(\[][ \t]\S)|(\S[ \t][\)\]])', "gratuitous whitespace in () or []"), | ||
|
r10281 | # (r'\s\s=', "gratuitous whitespace before ="), | ||
|
r17167 | (r'[^>< ](\+=|-=|!=|<>|<=|>=|<<=|>>=|%=)\S', | ||
|
r11345 | "missing whitespace around operator"), | ||
|
r17167 | (r'[^>< ](\+=|-=|!=|<>|<=|>=|<<=|>>=|%=)\s', | ||
|
r11345 | "missing whitespace around operator"), | ||
|
r17167 | (r'\s(\+=|-=|!=|<>|<=|>=|<<=|>>=|%=)\S', | ||
|
r11345 | "missing whitespace around operator"), | ||
|
r17167 | (r'[^^+=*/!<>&| %-](\s=|=\s)[^= ]', | ||
|
r11345 | "wrong whitespace around ="), | ||
|
r19872 | (r'\([^()]*( =[^=]|[^<>!=]= )', | ||
"no whitespace around = for named parameters"), | ||||
|
r18180 | (r'raise [^,(]+, (\([^\)]+\)|[^,\(\)]+)$', | ||
"don't use old-style two-argument raise, use Exception(message)"), | ||||
|
r14009 | (r' is\s+(not\s+)?["\'0-9-]', "object comparison with literal"), | ||
(r' [=!]=\s+(True|False|None)', | ||||
"comparison with singleton, use 'is' or 'is not' instead"), | ||||
|
r14494 | (r'^\s*(while|if) [01]:', | ||
"use True/False for constant Boolean expression"), | ||||
|
r33369 | (r'^\s*if False(:| +and)', 'Remove code instead of using `if False`'), | ||
|
r29796 | (r'(?:(?<!def)\s+|\()hasattr\(', | ||
|
r32418 | 'hasattr(foo, bar) is broken on py2, use util.safehasattr(foo, bar) ' | ||
'instead', r'#.*hasattr-py3-only'), | ||||
|
r14169 | (r'opener\([^)]*\).read\(', | ||
"use opener.read() instead"), | ||||
(r'opener\([^)]*\).write\(', | ||||
"use opener.write() instead"), | ||||
|
r23139 | (r'(?i)descend[e]nt', "the proper spelling is descendAnt"), | ||
|
r14709 | (r'\.debug\(\_', "don't mark debug messages for translation"), | ||
|
r16590 | (r'\.strip\(\)\.split\(\)', "no need to strip before splitting"), | ||
|
r18762 | (r'^\s*except\s*:', "naked except clause", r'#.*re-raises'), | ||
|
r25661 | (r'^\s*except\s([^\(,]+|\([^\)]+\))\s*,', | ||
'legacy exception syntax; use "as" instead of ","'), | ||||
|
r19031 | (r'release\(.*wlock, .*lock\)', "wrong lock release order"), | ||
|
r31476 | (r'\bdef\s+__bool__\b', "__bool__ should be __nonzero__ in Python 2"), | ||
|
r24836 | (r'os\.path\.join\(.*, *(""|\'\')\)', | ||
"use pathutil.normasprefix(path) instead of os.path.join(path, '')"), | ||||
|
r25659 | (r'\s0[0-7]+\b', 'legacy octal syntax; use "0o" prefix instead of "0"'), | ||
|
r26348 | # XXX only catch mutable arguments on the first line of the definition | ||
(r'def.*[( ]\w+=\{\}', "don't use mutable default arguments"), | ||||
|
r26588 | (r'\butil\.Abort\b', "directly use error.Abort"), | ||
|
r30810 | (r'^@(\w*\.)?cachefunc', "module-level @cachefunc is risky, please avoid"), | ||
|
r37863 | (r'^import Queue', "don't use Queue, use pycompat.queue.Queue + " | ||
"pycompat.queue.Empty"), | ||||
|
r28884 | (r'^import cStringIO', "don't use cStringIO.StringIO, use util.stringio"), | ||
(r'^import urllib', "don't use urllib, use util.urlreq/util.urlerr"), | ||||
|
r29434 | (r'^import SocketServer', "don't use SockerServer, use util.socketserver"), | ||
|
r31572 | (r'^import urlparse', "don't use urlparse, use util.urlreq"), | ||
|
r29434 | (r'^import xmlrpclib', "don't use xmlrpclib, use util.xmlrpclib"), | ||
(r'^import cPickle', "don't use cPickle, use util.pickle"), | ||||
(r'^import pickle', "don't use pickle, use util.pickle"), | ||||
|
r29455 | (r'^import httplib', "don't use httplib, use util.httplib"), | ||
|
r29566 | (r'^import BaseHTTPServer', "use util.httpserver instead"), | ||
|
r32599 | (r'^(from|import) mercurial\.(cext|pure|cffi)', | ||
"use mercurial.policy.importmod instead"), | ||||
|
r29217 | (r'\.next\(\)', "don't use .next(), use next(...)"), | ||
|
r31721 | (r'([a-z]*).revision\(\1\.node\(', | ||
|
r31786 | "don't convert rev to node before passing to revision(nodeorrev)"), | ||
|
r34643 | (r'platform\.system\(\)', "don't use platform.system(), use pycompat"), | ||
|
r29278 | |||
|
r41987 | ], | ||
# warnings | ||||
[ | ||||
] | ||||
] | ||||
# patterns to check normal *.py files | ||||
pypats = [ | ||||
[ | ||||
# Ideally, these should be placed in "commonpypats" for | ||||
# consistency of coding rules in Mercurial source tree. | ||||
# But on the other hand, these are not so seriously required for | ||||
# python code fragments embedded in test scripts. Fixing test | ||||
# scripts for these patterns requires many changes, and has less | ||||
# profit than effort. | ||||
(r'.{81}', "line too long"), | ||||
(r'raise Exception', "don't raise generic exceptions"), | ||||
(r'[\s\(](open|file)\([^)]*\)\.read\(', | ||||
"use util.readfile() instead"), | ||||
(r'[\s\(](open|file)\([^)]*\)\.write\(', | ||||
"use util.writefile() instead"), | ||||
(r'^[\s\(]*(open(er)?|file)\([^)]*\)(?!\.close\(\))', | ||||
"always assign an opened file to a variable, and close it afterwards"), | ||||
(r'[\s\(](open|file)\([^)]*\)\.(?!close\(\))', | ||||
"always assign an opened file to a variable, and close it afterwards"), | ||||
(r':\n( )*( ){1,3}[^ ]', "must indent 4 spaces"), | ||||
(r'^import atexit', "don't use atexit, use ui.atexit"), | ||||
|
r29278 | # rules depending on implementation of repquote() | ||
|
r29279 | (r' x+[xpqo%APM][\'"]\n\s+[\'"]x', | ||
'string join across lines with no space'), | ||||
|
r29397 | (r'''(?x)ui\.(status|progress|write|note|warn)\( | ||
[ \t\n#]* | ||||
(?# any strings/comments might precede a string, which | ||||
# contains translatable message) | ||||
((['"]|\'\'\'|""")[ \npq%bAPMxno]*(['"]|\'\'\'|""")[ \t\n#]+)* | ||||
(?# sequence consisting of below might precede translatable message | ||||
# - formatting string: "% 10s", "%05d", "% -3.2f", "%*s", "%%" ... | ||||
# - escaped character: "\\", "\n", "\0" ... | ||||
# - character other than '%', 'b' as '\', and 'x' as alphabet) | ||||
(['"]|\'\'\'|""") | ||||
((%([ n]?[PM]?([np]+|A))?x)|%%|b[bnx]|[ \nnpqAPMo])*x | ||||
(?# this regexp can't use [^...] style, | ||||
# because _preparepats forcibly adds "\n" into [^...], | ||||
# even though this regexp wants match it against "\n")''', | ||||
|
r29278 | "missing _() in ui message (use () to hide false-positives)"), | ||
|
r41987 | ] + commonpypats[0], | ||
|
r14009 | # warnings | ||
[ | ||||
|
r29278 | # rules depending on implementation of repquote() | ||
|
r19999 | (r'(^| )pp +xxxxqq[ \n][^\n]', "add two newlines after '.. note::'"), | ||
|
r41987 | ] + commonpypats[1] | ||
|
r10281 | ] | ||
|
r41992 | # patterns to check *.py for embedded ones in test script | ||
embeddedpypats = [ | ||||
[ | ||||
] + commonpypats[0], | ||||
# warnings | ||||
[ | ||||
] + commonpypats[1] | ||||
] | ||||
|
r41987 | # common filters to convert *.py | ||
commonpyfilters = [ | ||||
|
r10727 | (r"""(?msx)(?P<comment>\#.*?$)| | ||
((?P<quote>('''|\"\"\"|(?<!')'(?!')|(?<!")"(?!"))) | ||||
(?P<text>(([^\\]|\\.)*?)) | ||||
(?P=quote))""", reppython), | ||||
|
r10281 | ] | ||
|
r41987 | # filters to convert normal *.py files | ||
pyfilters = [ | ||||
] + commonpyfilters | ||||
|
r34649 | # non-filter patterns | ||
pynfpats = [ | ||||
[ | ||||
(r'pycompat\.osname\s*[=!]=\s*[\'"]nt[\'"]', "use pycompat.iswindows"), | ||||
(r'pycompat\.osname\s*[=!]=\s*[\'"]posix[\'"]', "use pycompat.isposix"), | ||||
(r'pycompat\.sysplatform\s*[!=]=\s*[\'"]darwin[\'"]', | ||||
"use pycompat.isdarwin"), | ||||
], | ||||
# warnings | ||||
[], | ||||
] | ||||
|
r41992 | # filters to convert *.py for embedded ones in test script | ||
embeddedpyfilters = [ | ||||
] + commonpyfilters | ||||
|
r31602 | # extension non-filter patterns | ||
pyextnfpats = [ | ||||
[(r'^"""\n?[A-Z]', "don't capitalize docstring title")], | ||||
# warnings | ||||
[], | ||||
] | ||||
|
r18960 | txtfilters = [] | ||
txtpats = [ | ||||
[ | ||||
|
r41685 | (r'\s$', 'trailing whitespace'), | ||
|
r20532 | ('.. note::[ \n][^\n]', 'add two newlines after note::') | ||
|
r18960 | ], | ||
[] | ||||
] | ||||
|
r10281 | cpats = [ | ||
|
r14009 | [ | ||
|
r10281 | (r'//', "don't use //-style comments"), | ||
(r'\S\t', "don't use tabs except for indent"), | ||||
|
r15281 | (r'(\S[ \t]+|^[ \t]+)\n', "trailing whitespace"), | ||
|
r16702 | (r'.{81}', "line too long"), | ||
|
r10281 | (r'(while|if|do|for)\(', "use space after while/if/do/for"), | ||
(r'return\(', "return is not a function"), | ||||
(r' ;', "no space before ;"), | ||||
|
r24453 | (r'[^;] \)', "no space before )"), | ||
|
r19745 | (r'[)][{]', "space between ) and {"), | ||
|
r10281 | (r'\w+\* \w+', "use int *foo, not int* foo"), | ||
|
r19731 | (r'\W\([^\)]+\) \w+', "use (int)foo, not (int) foo"), | ||
|
r16413 | (r'\w+ (\+\+|--)', "use foo++, not foo ++"), | ||
|
r10281 | (r'\w,\w', "missing whitespace after ,"), | ||
|
r13736 | (r'^[^#]\w[+/*]\w', "missing whitespace in expression"), | ||
|
r27640 | (r'\w\s=\s\s+\w', "gratuitous whitespace after ="), | ||
|
r10281 | (r'^#\s+\w', "use #foo, not # foo"), | ||
(r'[^\n]\Z', "no trailing newline"), | ||||
|
r13748 | (r'^\s*#import\b', "use only #include in standard C code"), | ||
|
r28594 | (r'strcpy\(', "don't use strcpy, use strlcpy or memcpy"), | ||
|
r28595 | (r'strcat\(', "don't use strcat"), | ||
|
r29278 | |||
# rules depending on implementation of repquote() | ||||
|
r14009 | ], | ||
# warnings | ||||
|
r29278 | [ | ||
# rules depending on implementation of repquote() | ||||
] | ||||
|
r10281 | ] | ||
cfilters = [ | ||||
(r'(/\*)(((\*(?!/))|[^*])*)\*/', repccomment), | ||||
|
r10722 | (r'''(?P<quote>(?<!")")(?P<text>([^"]|\\")+)"(?!")''', repquote), | ||
|
r10281 | (r'''(#\s*include\s+<)([^>]+)>''', repinclude), | ||
(r'(\()([^)]+\))', repcallspaces), | ||||
] | ||||
|
r14137 | inutilpats = [ | ||
[ | ||||
(r'\bui\.', "don't use ui in util"), | ||||
], | ||||
# warnings | ||||
[] | ||||
] | ||||
inrevlogpats = [ | ||||
[ | ||||
(r'\brepo\.', "don't use repo in revlog"), | ||||
], | ||||
# warnings | ||||
[] | ||||
] | ||||
|
r21487 | webtemplatefilters = [] | ||
webtemplatepats = [ | ||||
[], | ||||
[ | ||||
(r'{desc(\|(?!websub|firstline)[^\|]*)+}', | ||||
'follow desc keyword with either firstline or websub'), | ||||
] | ||||
] | ||||
|
r30246 | allfilesfilters = [] | ||
allfilespats = [ | ||||
[ | ||||
(r'(http|https)://[a-zA-Z0-9./]*selenic.com/', | ||||
'use mercurial-scm.org domain URL'), | ||||
|
r30888 | (r'mercurial@selenic\.com', | ||
'use mercurial-scm.org domain for mercurial ML address'), | ||||
|
r30890 | (r'mercurial-devel@selenic\.com', | ||
'use mercurial-scm.org domain for mercurial-devel ML address'), | ||||
|
r30246 | ], | ||
# warnings | ||||
[], | ||||
] | ||||
|
r30665 | py3pats = [ | ||
[ | ||||
|
r32185 | (r'os\.environ', "use encoding.environ instead (py3)", r'#.*re-exports'), | ||
|
r30665 | (r'os\.name', "use pycompat.osname instead (py3)"), | ||
|
r39843 | (r'os\.getcwd', "use encoding.getcwd instead (py3)", r'#.*re-exports'), | ||
|
r30665 | (r'os\.sep', "use pycompat.ossep instead (py3)"), | ||
(r'os\.pathsep', "use pycompat.ospathsep instead (py3)"), | ||||
(r'os\.altsep', "use pycompat.osaltsep instead (py3)"), | ||||
(r'sys\.platform', "use pycompat.sysplatform instead (py3)"), | ||||
(r'getopt\.getopt', "use pycompat.getoptb instead (py3)"), | ||||
|
r30820 | (r'os\.getenv', "use encoding.environ.get instead"), | ||
(r'os\.setenv', "modifying the environ dict is not preferred"), | ||||
|
r38807 | (r'(?<!pycompat\.)xrange', "use pycompat.xrange instead (py3)"), | ||
|
r30665 | ], | ||
# warnings | ||||
[], | ||||
] | ||||
|
r10281 | checks = [ | ||
|
r21222 | ('python', r'.*\.(py|cgi)$', r'^#!.*python', pyfilters, pypats), | ||
|
r34649 | ('python', r'.*\.(py|cgi)$', r'^#!.*python', [], pynfpats), | ||
|
r31602 | ('python', r'.*hgext.*\.py$', '', [], pyextnfpats), | ||
|
r32184 | ('python 3', r'.*(hgext|mercurial)/(?!demandimport|policy|pycompat).*\.py', | ||
'', pyfilters, py3pats), | ||||
|
r21222 | ('test script', r'(.*/)?test-[^.~]*$', '', testfilters, testpats), | ||
('c', r'.*\.[ch]$', '', cfilters, cpats), | ||||
('unified test', r'.*\.t$', '', utestfilters, utestpats), | ||||
('layering violation repo in revlog', r'mercurial/revlog\.py', '', | ||||
pyfilters, inrevlogpats), | ||||
('layering violation ui in util', r'mercurial/util\.py', '', pyfilters, | ||||
|
r14137 | inutilpats), | ||
|
r21222 | ('txt', r'.*\.txt$', '', txtfilters, txtpats), | ||
|
r21487 | ('web template', r'mercurial/templates/.*\.tmpl', '', | ||
webtemplatefilters, webtemplatepats), | ||||
|
r30246 | ('all except for .po', r'.*(?<!\.po)$', '', | ||
allfilesfilters, allfilespats), | ||||
|
r10281 | ] | ||
|
r41992 | # (desc, | ||
# func to pick up embedded code fragments, | ||||
# list of patterns to convert target files | ||||
# list of patterns to detect errors/warnings) | ||||
embeddedchecks = [ | ||||
('embedded python', | ||||
testparseutil.pyembedded, embeddedpyfilters, embeddedpypats) | ||||
] | ||||
|
r19307 | def _preparepats(): | ||
|
r41988 | def preparefailandwarn(failandwarn): | ||
|
r19307 | for pats in failandwarn: | ||
for i, pseq in enumerate(pats): | ||||
# fix-up regexes for multi-line searches | ||||
|
r19378 | p = pseq[0] | ||
|
r36975 | # \s doesn't match \n (done in two steps) | ||
# first, we replace \s that appears in a set already | ||||
p = re.sub(r'\[\\s', r'[ \\t', p) | ||||
# now we replace other \s instances. | ||||
p = re.sub(r'(?<!(\\|\[))\\s', r'[ \\t]', p) | ||||
|
r19307 | # [^...] doesn't match newline | ||
p = re.sub(r'(?<!\\)\[\^', r'[^\\n', p) | ||||
|
r19308 | pats[i] = (re.compile(p, re.MULTILINE),) + pseq[1:] | ||
|
r41988 | |||
def preparefilters(filters): | ||||
|
r19309 | for i, flt in enumerate(filters): | ||
filters[i] = re.compile(flt[0]), flt[1] | ||||
|
r19307 | |||
|
r41992 | for cs in (checks, embeddedchecks): | ||
|
r41988 | for c in cs: | ||
failandwarn = c[-1] | ||||
preparefailandwarn(failandwarn) | ||||
filters = c[-2] | ||||
preparefilters(filters) | ||||
|
r10719 | class norepeatlogger(object): | ||
def __init__(self): | ||||
self._lastseen = None | ||||
|
r11604 | def log(self, fname, lineno, line, msg, blame): | ||
|
r10719 | """print error related a to given line of a given file. | ||
The faulty line will also be printed but only once in the case | ||||
of multiple errors. | ||||
|
r10281 | |||
|
r10719 | :fname: filename | ||
:lineno: line number | ||||
:line: actual content of the line | ||||
:msg: error message | ||||
""" | ||||
msgid = fname, lineno, line | ||||
if msgid != self._lastseen: | ||||
|
r11604 | if blame: | ||
|
r28509 | print("%s:%d (%s):" % (fname, lineno, blame)) | ||
|
r11604 | else: | ||
|
r28509 | print("%s:%d:" % (fname, lineno)) | ||
print(" > %s" % line) | ||||
|
r10719 | self._lastseen = msgid | ||
|
r28509 | print(" " + msg) | ||
|
r10719 | |||
_defaultlogger = norepeatlogger() | ||||
|
r11604 | def getblame(f): | ||
lines = [] | ||||
for l in os.popen('hg annotate -un %s' % f): | ||||
start, line = l.split(':', 1) | ||||
user, rev = start.split() | ||||
lines.append((line[1:-1], user, rev)) | ||||
return lines | ||||
def checkfile(f, logfunc=_defaultlogger.log, maxerr=None, warnings=False, | ||||
|
r15502 | blame=False, debug=False, lineno=True): | ||
|
r10719 | """checks style and portability of a given file | ||
:f: filepath | ||||
:logfunc: function used to report error | ||||
logfunc(filename, linenumber, linecontent, errormessage) | ||||
|
r17424 | :maxerr: number of error to display before aborting. | ||
|
r15873 | Set to false (default) to report all errors | ||
|
r10720 | |||
return True if no error is found, False otherwise. | ||||
|
r10719 | """ | ||
|
r10720 | result = True | ||
|
r21222 | |||
try: | ||||
|
r29145 | with opentext(f) as fp: | ||
try: | ||||
|
r41401 | pre = fp.read() | ||
|
r29145 | except UnicodeDecodeError as e: | ||
print("%s while reading %s" % (e, f)) | ||||
return result | ||||
|
r25660 | except IOError as e: | ||
|
r28509 | print("Skipping %s, %s" % (f, str(e).split(':', 1)[0])) | ||
|
r21222 | return result | ||
|
r41989 | # context information shared while single checkfile() invocation | ||
context = {'blamecache': None} | ||||
|
r21222 | for name, match, magic, filters, pats in checks: | ||
|
r14135 | if debug: | ||
|
r28509 | print(name, f) | ||
|
r28050 | if not (re.match(match, f) or (magic and re.search(magic, pre))): | ||
|
r14135 | if debug: | ||
|
r28509 | print("Skipping %s for %s it doesn't match %s" % ( | ||
name, match, f)) | ||||
|
r10281 | continue | ||
|
r19382 | if "no-" "check-code" in pre: | ||
|
r27560 | # If you're looking at this line, it's because a file has: | ||
# no- check- code | ||||
# but the reason to output skipping is to make life for | ||||
# tests easier. So, instead of writing it with a normal | ||||
# spelling, we write it with the expected spelling from | ||||
# tests/test-check-code.t | ||||
|
r28509 | print("Skipping %s it has no-che?k-code (glob)" % f) | ||
|
r20239 | return "Skip" # skip checking this file | ||
|
r41989 | |||
|
r41990 | fc = _checkfiledata(name, f, pre, filters, pats, context, | ||
logfunc, maxerr, warnings, blame, debug, lineno) | ||||
if fc: | ||||
|
r41989 | result = False | ||
|
r41992 | if f.endswith('.t') and "no-" "check-code" not in pre: | ||
if debug: | ||||
print("Checking embedded code in %s" % (f)) | ||||
prelines = pre.splitlines() | ||||
embeddederros = [] | ||||
for name, embedded, filters, pats in embeddedchecks: | ||||
# "reset curmax at each repetition" treats maxerr as "max | ||||
# nubmer of errors in an actual file per entry of | ||||
# (embedded)checks" | ||||
curmaxerr = maxerr | ||||
for found in embedded(f, prelines, embeddederros): | ||||
filename, starts, ends, code = found | ||||
fc = _checkfiledata(name, f, code, filters, pats, context, | ||||
logfunc, curmaxerr, warnings, blame, debug, | ||||
lineno, offset=starts - 1) | ||||
if fc: | ||||
result = False | ||||
if curmaxerr: | ||||
if fc >= curmaxerr: | ||||
break | ||||
curmaxerr -= fc | ||||
|
r41989 | return result | ||
def _checkfiledata(name, f, filedata, filters, pats, context, | ||||
|
r41991 | logfunc, maxerr, warnings, blame, debug, lineno, | ||
offset=None): | ||||
|
r41989 | """Execute actual error check for file data | ||
:name: of the checking category | ||||
:f: filepath | ||||
:filedata: content of a file | ||||
:filters: to be applied before checking | ||||
:pats: to detect errors | ||||
:context: a dict of information shared while single checkfile() invocation | ||||
Valid keys: 'blamecache'. | ||||
:logfunc: function used to report error | ||||
logfunc(filename, linenumber, linecontent, errormessage) | ||||
:maxerr: number of error to display before aborting, or False to | ||||
report all errors | ||||
:warnings: whether warning level checks should be applied | ||||
:blame: whether blame information should be displayed at error reporting | ||||
:debug: whether debug information should be displayed | ||||
:lineno: whether lineno should be displayed at error reporting | ||||
|
r41991 | :offset: line number offset of 'filedata' in 'f' for checking | ||
an embedded code fragment, or None (offset=0 is different | ||||
from offset=None) | ||||
|
r41989 | |||
|
r41990 | returns number of detected errors. | ||
|
r41989 | """ | ||
blamecache = context['blamecache'] | ||||
|
r41991 | if offset is None: | ||
lineoffset = 0 | ||||
else: | ||||
lineoffset = offset | ||||
|
r41989 | |||
fc = 0 | ||||
pre = post = filedata | ||||
if True: # TODO: get rid of this redundant 'if' block | ||||
|
r10281 | for p, r in filters: | ||
post = re.sub(p, r, post) | ||||
|
r19422 | nerrs = len(pats[0]) # nerr elements are errors | ||
|
r14009 | if warnings: | ||
pats = pats[0] + pats[1] | ||||
else: | ||||
pats = pats[0] | ||||
|
r10281 | # print post # uncomment to show filtered version | ||
|
r15281 | |||
|
r14135 | if debug: | ||
|
r28509 | print("Checking %s for %s" % (name, f)) | ||
|
r15281 | |||
prelines = None | ||||
errors = [] | ||||
|
r19422 | for i, pat in enumerate(pats): | ||
|
r16705 | if len(pat) == 3: | ||
p, msg, ignore = pat | ||||
else: | ||||
p, msg = pat | ||||
ignore = None | ||||
|
r20005 | if i >= nerrs: | ||
msg = "warning: " + msg | ||||
|
r16705 | |||
|
r15281 | pos = 0 | ||
n = 0 | ||||
|
r19308 | for m in p.finditer(post): | ||
|
r15281 | if prelines is None: | ||
prelines = pre.splitlines() | ||||
postlines = post.splitlines(True) | ||||
start = m.start() | ||||
while n < len(postlines): | ||||
step = len(postlines[n]) | ||||
if pos + step > start: | ||||
break | ||||
pos += step | ||||
n += 1 | ||||
l = prelines[n] | ||||
|
r20242 | if ignore and re.search(ignore, l, re.MULTILINE): | ||
|
r20243 | if debug: | ||
|
r28509 | print("Skipping %s for %s:%s (ignore pattern)" % ( | ||
|
r41991 | name, f, (n + lineoffset))) | ||
|
r16705 | continue | ||
|
r15281 | bd = "" | ||
if blame: | ||||
bd = 'working directory' | ||||
|
r41989 | if blamecache is None: | ||
|
r15281 | blamecache = getblame(f) | ||
|
r41989 | context['blamecache'] = blamecache | ||
|
r41991 | if (n + lineoffset) < len(blamecache): | ||
bl, bu, br = blamecache[(n + lineoffset)] | ||||
if offset is None and bl == l: | ||||
|
r15281 | bd = '%s@%s' % (bu, br) | ||
|
r41991 | elif offset is not None and bl.endswith(l): | ||
# "offset is not None" means "checking | ||||
# embedded code fragment". In this case, | ||||
# "l" does not have information about the | ||||
# beginning of an *original* line in the | ||||
# file (e.g. ' > '). | ||||
# Therefore, use "str.endswith()", and | ||||
# show "maybe" for a little loose | ||||
# examination. | ||||
bd = '%s@%s, maybe' % (bu, br) | ||||
|
r20005 | |||
|
r41991 | errors.append((f, lineno and (n + lineoffset + 1), l, msg, bd)) | ||
|
r15281 | |||
errors.sort() | ||||
for e in errors: | ||||
logfunc(*e) | ||||
fc += 1 | ||||
|
r15873 | if maxerr and fc >= maxerr: | ||
|
r28509 | print(" (too many errors, giving up)") | ||
|
r10281 | break | ||
|
r15281 | |||
|
r41990 | return fc | ||
|
r10717 | |||
|
r29568 | def main(): | ||
|
r31824 | parser = optparse.OptionParser("%prog [options] [files | -]") | ||
|
r10895 | parser.add_option("-w", "--warnings", action="store_true", | ||
help="include warning-level checks") | ||||
parser.add_option("-p", "--per-file", type="int", | ||||
help="max warnings per file") | ||||
|
r11604 | parser.add_option("-b", "--blame", action="store_true", | ||
help="use annotate to generate blame info") | ||||
|
r14135 | parser.add_option("", "--debug", action="store_true", | ||
help="show debug information") | ||||
|
r15502 | parser.add_option("", "--nolineno", action="store_false", | ||
dest='lineno', help="don't show line numbers") | ||||
|
r10895 | |||
|
r15502 | parser.set_defaults(per_file=15, warnings=False, blame=False, debug=False, | ||
lineno=True) | ||||
|
r10895 | (options, args) = parser.parse_args() | ||
if len(args) == 0: | ||||
|
r10716 | check = glob.glob("*") | ||
|
r31824 | elif args == ['-']: | ||
# read file list from stdin | ||||
check = sys.stdin.read().splitlines() | ||||
|
r10716 | else: | ||
|
r10895 | check = args | ||
|
r10281 | |||
|
r29569 | _preparepats() | ||
|
r15544 | ret = 0 | ||
|
r10716 | for f in check: | ||
|
r11816 | if not checkfile(f, maxerr=options.per_file, warnings=options.warnings, | ||
|
r15502 | blame=options.blame, debug=options.debug, | ||
lineno=options.lineno): | ||||
|
r11816 | ret = 1 | ||
|
r29568 | return ret | ||
if __name__ == "__main__": | ||||
sys.exit(main()) | ||||