summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteve Reinhardt <steve.reinhardt@amd.com>2016-02-06 17:21:18 -0800
committerSteve Reinhardt <steve.reinhardt@amd.com>2016-02-06 17:21:18 -0800
commitc8c82f09a282832d919f7eb073a47be838e65b29 (patch)
tree57d2108fd5137a050f17e25422f760f3d543267b
parent9979355539fce2314495b421f79dcc86f95a6170 (diff)
downloadgem5-c8c82f09a282832d919f7eb073a47be838e65b29.tar.xz
util: clean up and extend style checker
Added a new Verifier object to check for and fix spacing between if/while/for and following paren. Restructured Verifier class to make it easier to add new subclasses, particularly by using a global list of verifiers to auto-generate command line options and simplify the invocation loop.
-rw-r--r--util/style.py196
1 files changed, 152 insertions, 44 deletions
diff --git a/util/style.py b/util/style.py
index bf73bc425..ee3c0bf94 100644
--- a/util/style.py
+++ b/util/style.py
@@ -13,6 +13,7 @@
#
# Copyright (c) 2006 The Regents of The University of Michigan
# Copyright (c) 2007,2011 The Hewlett-Packard Development Company
+# Copyright (c) 2016 Advanced Micro Devices, Inc.
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
@@ -39,6 +40,7 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#
# Authors: Nathan Binkert
+# Steve Reinhardt
import heapq
import os
@@ -62,8 +64,7 @@ all_regions = Regions(Region(neg_inf, pos_inf))
tabsize = 8
lead = re.compile(r'^([ \t]+)')
trail = re.compile(r'([ \t]+)$')
-any_control = re.compile(r'\b(if|while|for)[ \t]*[(]')
-good_control = re.compile(r'\b(if|while|for) [(]')
+any_control = re.compile(r'\b(if|while|for)([ \t]*)\(')
format_types = set(('C', 'C++'))
@@ -153,10 +154,31 @@ class StdioUI(UserInterface):
def write(self, string):
sys.stdout.write(string)
+
class Verifier(object):
- def __init__(self, ui, repo):
+ """Base class for style verifier objects
+
+ Subclasses must define these class attributes:
+ languages = set of strings identifying applicable languages
+ test_name = long descriptive name of test, will be used in
+ messages such as "error in <foo>" or "invalid <foo>"
+ opt_name = short name used to generate command-line options to
+ control the test (--fix-<foo>, --ignore-<foo>, etc.)
+ """
+
+ def __init__(self, ui, repo, opts):
self.ui = ui
self.repo = repo
+ # opt_name must be defined as a class attribute of derived classes.
+ # Check test-specific opts first as these have precedence.
+ self.opt_fix = opts.get('fix_' + self.opt_name, False)
+ self.opt_ignore = opts.get('ignore_' + self.opt_name, False)
+ self.opt_skip = opts.get('skip_' + self.opt_name, False)
+ # If no test-specific opts were set, then set based on "-all" opts.
+ if not (self.opt_fix or self.opt_ignore or self.opt_skip):
+ self.opt_fix = opts.get('fix_all', False)
+ self.opt_ignore = opts.get('ignore_all', False)
+ self.opt_skip = opts.get('skip_all', False)
def __getattr__(self, attr):
if attr in ('prompt', 'write'):
@@ -197,6 +219,17 @@ class Verifier(object):
return lang_type(filename) not in self.languages
def check(self, filename, regions=all_regions):
+ """Check specified regions of file 'filename'.
+
+ Line-by-line checks can simply provide a check_line() method
+ that returns True if the line is OK and False if it has an
+ error. Verifiers that need a multi-line view (like
+ SortedIncludes) must override this entire function.
+
+ Returns a count of errors (0 if none), though actual non-zero
+ count value is not currently used anywhere.
+ """
+
f = self.open(filename, 'r')
errors = 0
@@ -205,13 +238,20 @@ class Verifier(object):
continue
if not self.check_line(line):
self.write("invalid %s in %s:%d\n" % \
- (self.test_name, filename, num + 1))
+ (self.test_name, filename, num + 1))
if self.ui.verbose:
- self.write(">>%s<<\n" % line[-1])
+ self.write(">>%s<<\n" % line[:-1])
errors += 1
return errors
def fix(self, filename, regions=all_regions):
+ """Fix specified regions of file 'filename'.
+
+ Line-by-line fixes can simply provide a fix_line() method that
+ returns the fixed line. Verifiers that need a multi-line view
+ (like SortedIncludes) must override this entire function.
+ """
+
f = self.open(filename, 'r+')
lines = list(f)
@@ -226,18 +266,46 @@ class Verifier(object):
f.write(line)
f.close()
- def apply(self, filename, prompt, regions=all_regions):
- if not self.skip(filename):
+
+ def apply(self, filename, regions=all_regions):
+ """Possibly apply to specified regions of file 'filename'.
+
+ Verifier is skipped if --skip-<test> option was provided or if
+ file is not of an applicable type. Otherwise file is checked
+ and error messages printed. Errors are fixed or ignored if
+ the corresponding --fix-<test> or --ignore-<test> options were
+ provided. If neither, the user is prompted for an action.
+
+ Returns True to abort, False otherwise.
+ """
+ if not (self.opt_skip or self.skip(filename)):
errors = self.check(filename, regions)
- if errors:
- if prompt(filename, self.fix, regions):
- return True
+ if errors and not self.opt_ignore:
+ if self.opt_fix:
+ self.fix(filename, regions)
+ else:
+ result = self.ui.prompt("(a)bort, (i)gnore, or (f)ix?",
+ 'aif', 'a')
+ if result == 'f':
+ self.fix(filename, regions)
+ elif result == 'a':
+ return True # abort
+
return False
class Whitespace(Verifier):
+ """Check whitespace.
+
+ Specifically:
+ - No tabs used for indent
+ - No trailing whitespace
+ """
+
languages = set(('C', 'C++', 'swig', 'python', 'asm', 'isa', 'scons'))
test_name = 'whitespace'
+ opt_name = 'white'
+
def check_line(self, line):
match = lead.search(line)
if match and match.group(1).find('\t') != -1:
@@ -265,8 +333,30 @@ class Whitespace(Verifier):
return line.rstrip() + '\n'
+
+class ControlSpace(Verifier):
+ """Check for exactly one space after if/while/for"""
+
+ languages = set(('C', 'C++'))
+ test_name = 'spacing after if/while/for'
+ opt_name = 'control'
+
+ def check_line(self, line):
+ match = any_control.search(line)
+ return not (match and match.group(2) != " ")
+
+ def fix_line(self, line):
+ new_line = any_control.sub(r'\1 (', line)
+ return new_line
+
+
class SortedIncludes(Verifier):
+ """Check for proper sorting of include statements"""
+
languages = sort_includes.default_languages
+ test_name = 'include file order'
+ opt_name = 'include'
+
def __init__(self, *args, **kwargs):
super(SortedIncludes, self).__init__(*args, **kwargs)
self.sort_includes = sort_includes.SortIncludes()
@@ -314,6 +404,13 @@ class SortedIncludes(Verifier):
f.write('\n')
f.close()
+# list of all verifier classes
+all_verifiers = [
+ Whitespace,
+ ControlSpace,
+ SortedIncludes
+]
+
def linelen(line):
tabs = line.count('\t')
if not tabs:
@@ -411,7 +508,7 @@ def validate(filename, stats, verbose, exit_code):
# for c++, exactly one space betwen if/while/for and (
if lang == 'C++':
match = any_control.search(line)
- if match and not good_control.search(line):
+ if match and match.group(2) != " ":
stats.badcontrol += 1
if verbose > 1:
msg(i, line, 'improper spacing after %s' % match.group(1))
@@ -464,41 +561,41 @@ def do_check_style(hgui, repo, *pats, **opts):
The --all option can be specified to include clean files and check
modified files in their entirety.
- """
- opt_fix_all = opts.get('fix_all', False)
- if not opt_fix_all:
- opt_fix_white = opts.get('fix_white', False)
- opt_fix_include = opts.get('fix_include', False)
- else:
- opt_fix_white = True
- opt_fix_include = True
- ui = MercurialUI(hgui, verbose=hgui.verbose)
+ The --fix-<check>, --ignore-<check>, and --skip-<check> options
+ can be used to control individual style checks:
- def prompt(name, func, regions=all_regions):
- result = ui.prompt("(a)bort, (i)gnore, or (f)ix?", 'aif', 'a')
- if result == 'a':
- return True
- elif result == 'f':
- func(name, regions)
+ --fix-<check> will perform the check and automatically attempt to
+ fix sny style error (printing a warning if unsuccessful)
- return False
+ --ignore-<check> will perform the check but ignore any errors
+ found (other than printing a message for each)
- def no_prompt(name, func, regions=all_regions):
- func(name, regions)
- return False
+ --skip-<check> will skip performing the check entirely
- prompt_white = prompt if not opt_fix_white else no_prompt
- prompt_include = prompt if not opt_fix_include else no_prompt
+ If none of these options are given, all checks will be performed
+ and the user will be prompted on how to handle each error.
- whitespace = Whitespace(ui, repo)
- sorted_includes = SortedIncludes(ui, repo)
- for fname, mod_regions in _modified_regions(repo, pats, **opts):
- if whitespace.apply(fname, prompt_white, mod_regions):
- return True
+ --fix-all, --ignore-all, and --skip-all are equivalent to specifying
+ --fix-<check>, --ignore-<check>, or --skip-<check> for all checks,
+ respectively. However, option settings for specific checks take
+ precedence. Thus --skip-all --fix-white can be used to skip every
+ check other than whitespace errors, which will be checked and
+ automatically fixed.
- if sorted_includes.apply(fname, prompt_include, mod_regions):
- return True
+ The -v/--verbose flag will display the offending line(s) as well
+ as their location.
+ """
+
+ ui = MercurialUI(hgui, verbose=hgui.verbose)
+
+ # instantiate varifier objects
+ verifiers = [v(ui, repo, opts) for v in all_verifiers]
+
+ for fname, mod_regions in _modified_regions(repo, pats, **opts):
+ for verifier in verifiers:
+ if verifier.apply(fname, mod_regions):
+ return True
return False
@@ -536,6 +633,8 @@ def check_hook(hooktype):
raise AttributeError, \
"This hook is not meant for %s" % hooktype
+# This function provides a hook that is called before transaction
+# commit and on qrefresh
def check_style(ui, repo, hooktype, **kwargs):
check_hook(hooktype)
args = {}
@@ -570,13 +669,22 @@ _common_region_options = [
('', 'no-ignore', False, _("ignore the style ignore list")),
]
+
+fix_opts = [('f', 'fix-all', False, _("fix all style errors"))] + \
+ [('', 'fix-' + v.opt_name, False,
+ _('fix errors in ' + v.test_name)) for v in all_verifiers]
+ignore_opts = [('', 'ignore-all', False, _("ignore all style errors"))] + \
+ [('', 'ignore-' + v.opt_name, False,
+ _('ignore errors in ' + v.test_name)) for v in all_verifiers]
+skip_opts = [('', 'skip-all', False, _("skip all style error checks"))] + \
+ [('', 'skip-' + v.opt_name, False,
+ _('skip checking for ' + v.test_name)) for v in all_verifiers]
+all_opts = fix_opts + ignore_opts + skip_opts
+
+
cmdtable = {
'^m5style' : (
- do_check_style, [
- ('f', 'fix-all', False, _("automatically fix style issues")),
- ('', 'fix-white', False, _("automatically fix white space issues")),
- ('', 'fix-include', False, _("automatically fix include ordering")),
- ] + _common_region_options + commands.walkopts,
+ do_check_style, all_opts + _common_region_options + commands.walkopts,
_('hg m5style [-a] [FILE]...')),
'^m5format' :
( do_check_format, [