diff options
Diffstat (limited to 'contrib/check-code.py')
| -rwxr-xr-x | contrib/check-code.py | 202 |
1 files changed, 68 insertions, 134 deletions
diff --git a/contrib/check-code.py b/contrib/check-code.py index b8f714c..464659e 100755 --- a/contrib/check-code.py +++ b/contrib/check-code.py @@ -13,7 +13,7 @@ import optparse def repquote(m): t = re.sub(r"\w", "x", m.group('text')) - t = re.sub(r"[^\s\nx]", "o", t) + t = re.sub(r"[^\sx]", "o", t) return m.group('quote') + t + m.group('quote') def reppython(m): @@ -43,44 +43,35 @@ def rephere(m): testpats = [ [ - (r'pushd|popd', "don't use 'pushd' or 'popd', use 'cd'"), - (r'\W\$?\(\([^\)\n]*\)\)', "don't use (()) or $(()), use 'expr'"), + (r'(pushd|popd)', "don't use 'pushd' or 'popd', use 'cd'"), + (r'\W\$?\(\([^\)]*\)\)', "don't use (()) or $(()), use 'expr'"), + (r'^function', "don't use 'function', use old style"), (r'grep.*-q', "don't use 'grep -q', redirect to /dev/null"), - (r'sed.*-i', "don't use 'sed -i', use a temporary file"), - (r'\becho\b.*\\n', "don't use 'echo \\n', use printf"), + (r'echo.*\\n', "don't use 'echo \\n', use printf"), (r'echo -n', "don't use 'echo -n', use printf"), - (r'(^| )wc[^|]*$\n(?!.*\(re\))', "filter wc output"), + (r'^diff.*-\w*N', "don't use 'diff -N'"), + (r'(^| )wc[^|]*$', "filter wc output"), (r'head -c', "don't use 'head -c', use 'dd'"), - (r'sha1sum', "don't use sha1sum, use $TESTDIR/md5sum.py"), (r'ls.*-\w*R', "don't use 'ls -R', use 'find'"), - (r'printf.*\\([1-9]|0\d)', "don't use 'printf \NNN', use Python"), + (r'printf.*\\\d\d\d', "don't use 'printf \NNN', use Python"), (r'printf.*\\x', "don't use printf \\x, use Python"), (r'\$\(.*\)', "don't use $(expr), use `expr`"), (r'rm -rf \*', "don't use naked rm -rf, target a directory"), (r'(^|\|\s*)grep (-\w\s+)*[^|]*[(|]\w', "use egrep for extended grep syntax"), (r'/bin/', "don't use explicit paths for tools"), + (r'\$PWD', "don't use $PWD, use `pwd`"), (r'[^\n]\Z', "no trailing newline"), (r'export.*=', "don't export and assign at once"), + ('^([^"\']|("[^"]*")|(\'[^\']*\'))*\\^', "^ must be quoted"), (r'^source\b', "don't use 'source', use '.'"), (r'touch -d', "don't use 'touch -d', use 'touch -t' instead"), - (r'ls +[^|\n-]+ +-', "options to 'ls' must come before filenames"), - (r'[^>\n]>\s*\$HGRCPATH', "don't overwrite $HGRCPATH, append to it"), - (r'^stop\(\)', "don't use 'stop' as a shell function name"), - (r'(\[|\btest\b).*-e ', "don't use 'test -e', use 'test -f'"), - (r'^alias\b.*=', "don't use alias, use a function"), - (r'if\s*!', "don't use '!' to negate exit status"), - (r'/dev/u?random', "don't use entropy, use /dev/zero"), - (r'do\s*true;\s*done', "don't use true as loop body, use sleep 0"), - (r'^( *)\t', "don't use tabs to indent"), + (r'ls\s+[^|-]+\s+-', "options to 'ls' must come before filenames"), + (r'[^>]>\s*\$HGRCPATH', "don't overwrite $HGRCPATH, append to it"), + (r'stop\(\)', "don't use 'stop' as a shell function name"), ], # warnings - [ - (r'^function', "don't use 'function', use old style"), - (r'^diff.*-\w*N', "don't use 'diff -N'"), - (r'\$PWD', "don't use $PWD, use `pwd`"), - (r'^([^"\'\n]|("[^"\n]*")|(\'[^\'\n]*\'))*\^', "^ must be quoted"), - ] + [] ] testfilters = [ @@ -89,19 +80,17 @@ testfilters = [ ] uprefix = r"^ \$ " +uprefixc = r"^ > " utestpats = [ [ - (r'^(\S| $ ).*(\S[ \t]+|^[ \t]+)\n', "trailing whitespace on non-output"), - (uprefix + r'.*\|\s*sed[^|>\n]*\n', - "use regex test output patterns instead of sed"), + (r'^(\S| $ ).*(\S\s+|^\s+)\n', "trailing whitespace on non-output"), + (uprefix + r'.*\|\s*sed', "use regex test output patterns instead of sed"), (uprefix + r'(true|exit 0)', "explicit zero exit unnecessary"), - (uprefix + r'.*(?<!\[)\$\?', "explicit exit code checks unnecessary"), + (uprefix + r'.*\$\?', "explicit exit code checks unnecessary"), (uprefix + r'.*\|\| echo.*(fail|error)', "explicit exit code checks unnecessary"), (uprefix + r'set -e', "don't use set -e"), - (uprefix + r'\s', "don't indent commands, use > for continued lines"), - (r'^ saved backup bundle to \$TESTTMP.*\.hg$', - "use (glob) to match Windows paths too"), + (uprefixc + r'( *)\t', "don't use tabs to indent"), ], # warnings [] @@ -109,10 +98,10 @@ utestpats = [ for i in [0, 1]: for p, m in testpats[i]: - if p.startswith(r'^'): - p = r"^ [$>] (%s)" % p[1:] + if p.startswith('^'): + p = uprefix + p[1:] else: - p = r"^ [$>] .*(%s)" % p + p = uprefix + p utestpats[i].append((p, m)) utestfilters = [ @@ -130,25 +119,18 @@ pypats = [ (r'\.has_key\b', "dict.has_key is not available in Python 3+"), (r'^\s*\t', "don't use tabs"), (r'\S;\s*\n', "semicolon"), - (r'[^_]_\("[^"]+"\s*%', "don't use % inside _()"), - (r"[^_]_\('[^']+'\s*%", "don't use % inside _()"), (r'\w,\w', "missing whitespace after ,"), (r'\w[+/*\-<>]\w', "missing whitespace in expression"), - (r'^\s+\w+=\w+[^,)\n]$', "missing whitespace in assignment"), - (r'(\s+)try:\n((?:\n|\1\s.*\n)+?)\1except.*?:\n' - r'((?:\n|\1\s.*\n)+?)\1finally:', 'no try/except/finally in Py2.4'), - (r'.{81}', "line too long"), - (r' x+[xo][\'"]\n\s+[\'"]x', 'string join across lines with no space'), + (r'^\s+\w+=\w+[^,)]$', "missing whitespace in assignment"), + (r'.{85}', "line too long"), (r'[^\n]\Z', "no trailing newline"), - (r'(\S[ \t]+|^[ \t]+)\n', "trailing whitespace"), -# (r'^\s+[^_ \n][^_. \n]+_[^_\n]+\s*=', -# "don't use underbars in identifiers"), - (r'^\s+(self\.)?[A-za-z][a-z0-9]+[A-Z]\w* = ', - "don't use camelcase in identifiers"), - (r'^\s*(if|while|def|class|except|try)\s[^[\n]*:\s*[^\\n]#\s]+', + (r'(\S\s+|^\s+)\n', "trailing whitespace"), +# (r'^\s+[^_ ][^_. ]+_[^_]+\s*=', "don't use underbars in identifiers"), +# (r'\w*[a-z][A-Z]\w*\s*=', "don't use camelcase in identifiers"), + (r'^\s*(if|while|def|class|except|try)\s[^[]*:\s*[^\]#\s]+', "linebreak after :"), - (r'class\s[^( \n]+:', "old-style class, use class foo(object)"), - (r'class\s[^( \n]+\(\):', + (r'class\s[^( ]+:', "old-style class, use class foo(object)"), + (r'class\s[^( ]+\(\):', "class foo() not available in Python 2.4, use class foo(object)"), (r'\b(%s)\(' % '|'.join(keyword.kwlist), "Python keyword is not a function"), @@ -166,19 +148,19 @@ pypats = [ (r'(?<!def)\s+(any|all|format)\(', "any/all/format not available in Python 2.4"), (r'(?<!def)\s+(callable)\(', - "callable not available in Python 3, use getattr(f, '__call__', None)"), + "callable not available in Python 3, use hasattr(f, '__call__')"), (r'if\s.*\selse', "if ... else form not available in Python 2.4"), (r'^\s*(%s)\s\s' % '|'.join(keyword.kwlist), "gratuitous whitespace after Python keyword"), - (r'([\(\[][ \t]\S)|(\S[ \t][\)\]])', "gratuitous whitespace in () or []"), + (r'([\(\[]\s\S)|(\S\s[\)\]])', "gratuitous whitespace in () or []"), # (r'\s\s=', "gratuitous whitespace before ="), - (r'[^>< ](\+=|-=|!=|<>|<=|>=|<<=|>>=|%=)\S', + (r'[^>< ](\+=|-=|!=|<>|<=|>=|<<=|>>=)\S', "missing whitespace around operator"), - (r'[^>< ](\+=|-=|!=|<>|<=|>=|<<=|>>=|%=)\s', + (r'[^>< ](\+=|-=|!=|<>|<=|>=|<<=|>>=)\s', "missing whitespace around operator"), - (r'\s(\+=|-=|!=|<>|<=|>=|<<=|>>=|%=)\S', + (r'\s(\+=|-=|!=|<>|<=|>=|<<=|>>=)\S', "missing whitespace around operator"), - (r'[^^+=*/!<>&| %-](\s=|=\s)[^= ]', + (r'[^+=*/!<>&| -](\s=|=\s)[^= ]', "wrong whitespace around ="), (r'raise Exception', "don't raise generic exceptions"), (r' is\s+(not\s+)?["\'0-9-]', "object comparison with literal"), @@ -186,12 +168,8 @@ pypats = [ "comparison with singleton, use 'is' or 'is not' instead"), (r'^\s*(while|if) [01]:', "use True/False for constant Boolean expression"), - (r'(?:(?<!def)\s+|\()hasattr', - 'hasattr(foo, bar) is broken, use util.safehasattr(foo, bar) instead'), (r'opener\([^)]*\).read\(', "use opener.read() instead"), - (r'BaseException', 'not in Py2.4, use Exception'), - (r'os\.path\.relpath', 'os.path.relpath is not in Py2.5'), (r'opener\([^)]*\).write\(', "use opener.write() instead"), (r'[\s\(](open|file)\([^)]*\)\.read\(', @@ -204,12 +182,11 @@ pypats = [ "always assign an opened file to a variable, and close it afterwards"), (r'(?i)descendent', "the proper spelling is descendAnt"), (r'\.debug\(\_', "don't mark debug messages for translation"), - (r'\.strip\(\)\.split\(\)', "no need to strip before splitting"), - (r'^\s*except\s*:', "warning: naked except clause", r'#.*re-raises'), - (r':\n( )*( ){1,3}[^ ]', "must indent 4 spaces"), ], # warnings [ + (r'.{81}', "warning: line over 80 characters"), + (r'^\s*except:$', "warning: naked except clause"), (r'ui\.(status|progress|write|note|warn)\([\'\"]x', "warning: unwrapped ui message"), ] @@ -227,14 +204,14 @@ cpats = [ (r'//', "don't use //-style comments"), (r'^ ', "don't use spaces to indent"), (r'\S\t', "don't use tabs except for indent"), - (r'(\S[ \t]+|^[ \t]+)\n', "trailing whitespace"), - (r'.{81}', "line too long"), + (r'(\S\s+|^\s+)\n', "trailing whitespace"), + (r'.{85}', "line too long"), (r'(while|if|do|for)\(', "use space after while/if/do/for"), (r'return\(', "return is not a function"), (r' ;', "no space before ;"), (r'\w+\* \w+', "use int *foo, not int* foo"), (r'\([^\)]+\) \w+', "use (int)foo, not (int) foo"), - (r'\w+ (\+\+|--)', "use foo++, not foo ++"), + (r'\S+ (\+\+|--)', "use foo++, not foo ++"), (r'\w,\w', "missing whitespace after ,"), (r'^[^#]\w[+/*]\w', "missing whitespace in expression"), (r'^#\s+\w', "use #foo, not # foo"), @@ -315,14 +292,14 @@ def getblame(f): return lines def checkfile(f, logfunc=_defaultlogger.log, maxerr=None, warnings=False, - blame=False, debug=False, lineno=True): + blame=False, debug=False): """checks style and portability of a given file :f: filepath :logfunc: function used to report error logfunc(filename, linenumber, linecontent, errormessage) :maxerr: number of error to display before arborting. - Set to false (default) to report all errors + Set to None (default) to report all errors return True if no error is found, False otherwise. """ @@ -352,71 +329,32 @@ def checkfile(f, logfunc=_defaultlogger.log, maxerr=None, warnings=False, else: pats = pats[0] # print post # uncomment to show filtered version - + z = enumerate(zip(pre.splitlines(), post.splitlines(True))) if debug: print "Checking %s for %s" % (name, f) - - prelines = None - errors = [] - for pat in pats: - if len(pat) == 3: - p, msg, ignore = pat - else: - p, msg = pat - ignore = None - - # fix-up regexes for multiline searches - po = p - # \s doesn't match \n - p = re.sub(r'(?<!\\)\\s', r'[ \\t]', p) - # [^...] doesn't match newline - p = re.sub(r'(?<!\\)\[\^', r'[^\\n', p) - - #print po, '=>', p - - pos = 0 - n = 0 - for m in re.finditer(p, post, re.MULTILINE): - 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] - - if "check-code" + "-ignore" in l: - if debug: - print "Skipping %s for %s:%s (check-code -ignore)" % ( - name, f, n) - continue - elif ignore and re.search(ignore, l, re.MULTILINE): - continue - bd = "" - if blame: - bd = 'working directory' - if not blamecache: - blamecache = getblame(f) - if n < len(blamecache): - bl, bu, br = blamecache[n] - if bl == l: - bd = '%s@%s' % (bu, br) - errors.append((f, lineno and n + 1, l, msg, bd)) - result = False - - errors.sort() - for e in errors: - logfunc(*e) - fc += 1 - if maxerr and fc >= maxerr: + for n, l in z: + if "check-code" + "-ignore" in l[0]: + if debug: + print "Skipping %s for %s:%s (check-code -ignore)" % ( + name, f, n) + continue + for p, msg in pats: + if re.search(p, l[1]): + bd = "" + if blame: + bd = 'working directory' + if not blamecache: + blamecache = getblame(f) + if n < len(blamecache): + bl, bu, br = blamecache[n] + if bl == l[0]: + bd = '%s@%s' % (bu, br) + logfunc(f, n + 1, l[0], msg, bd) + fc += 1 + result = False + if maxerr is not None and fc >= maxerr: print " (too many errors, giving up)" break - return result if __name__ == "__main__": @@ -429,11 +367,8 @@ if __name__ == "__main__": help="use annotate to generate blame info") parser.add_option("", "--debug", action="store_true", help="show debug information") - parser.add_option("", "--nolineno", action="store_false", - dest='lineno', help="don't show line numbers") - parser.set_defaults(per_file=15, warnings=False, blame=False, debug=False, - lineno=True) + parser.set_defaults(per_file=15, warnings=False, blame=False, debug=False) (options, args) = parser.parse_args() if len(args) == 0: @@ -441,10 +376,9 @@ if __name__ == "__main__": else: check = args - ret = 0 for f in check: + ret = 0 if not checkfile(f, maxerr=options.per_file, warnings=options.warnings, - blame=options.blame, debug=options.debug, - lineno=options.lineno): + blame=options.blame, debug=options.debug): ret = 1 sys.exit(ret) |