Merge "Improved tools/hacking.py"
This commit is contained in:
commit
d10077d18e
@ -82,7 +82,7 @@ Example::
|
|||||||
"""A one line docstring looks like this and ends in a period."""
|
"""A one line docstring looks like this and ends in a period."""
|
||||||
|
|
||||||
|
|
||||||
"""A multiline docstring has a one-line summary, less than 80 characters.
|
"""A multi line docstring has a one-line summary, less than 80 characters.
|
||||||
|
|
||||||
Then a new paragraph after a newline that explains in more detail any
|
Then a new paragraph after a newline that explains in more detail any
|
||||||
general information about the function, class or method. Example usages
|
general information about the function, class or method. Example usages
|
||||||
|
@ -1038,7 +1038,7 @@ class LibvirtConnection(driver.ComputeDriver):
|
|||||||
finally:
|
finally:
|
||||||
try:
|
try:
|
||||||
os.unlink(testfile)
|
os.unlink(testfile)
|
||||||
except:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
return hasDirectIO
|
return hasDirectIO
|
||||||
|
@ -108,7 +108,10 @@ function run_pep8 {
|
|||||||
echo "Running PEP8 and HACKING compliance check..."
|
echo "Running PEP8 and HACKING compliance check..."
|
||||||
# Just run PEP8 in current environment
|
# Just run PEP8 in current environment
|
||||||
#
|
#
|
||||||
${wrapper} python tools/hacking.py ${srcfiles}
|
|
||||||
|
# Until all these issues get fixed, ignore.
|
||||||
|
ignore='--ignore=N4,N306'
|
||||||
|
${wrapper} python tools/hacking.py ${ignore} ${srcfiles}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
153
tools/hacking.py
153
tools/hacking.py
@ -22,14 +22,18 @@ built on top of pep8.py
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import inspect
|
import inspect
|
||||||
|
import logging
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
import sys
|
import sys
|
||||||
import tokenize
|
import tokenize
|
||||||
import traceback
|
import traceback
|
||||||
|
import warnings
|
||||||
|
|
||||||
import pep8
|
import pep8
|
||||||
|
|
||||||
|
# Don't need this for testing
|
||||||
|
logging.disable('LOG')
|
||||||
|
|
||||||
#N1xx comments
|
#N1xx comments
|
||||||
#N2xx except
|
#N2xx except
|
||||||
@ -40,6 +44,8 @@ import pep8
|
|||||||
#N7xx localization
|
#N7xx localization
|
||||||
|
|
||||||
IMPORT_EXCEPTIONS = ['sqlalchemy', 'migrate', 'nova.db.sqlalchemy.session']
|
IMPORT_EXCEPTIONS = ['sqlalchemy', 'migrate', 'nova.db.sqlalchemy.session']
|
||||||
|
DOCSTRING_TRIPLE = ['"""', "'''"]
|
||||||
|
VERBOSE_MISSING_IMPORT = False
|
||||||
|
|
||||||
|
|
||||||
def is_import_exception(mod):
|
def is_import_exception(mod):
|
||||||
@ -47,8 +53,24 @@ def is_import_exception(mod):
|
|||||||
any(mod.startswith(m + '.') for m in IMPORT_EXCEPTIONS)
|
any(mod.startswith(m + '.') for m in IMPORT_EXCEPTIONS)
|
||||||
|
|
||||||
|
|
||||||
|
def import_normalize(line):
|
||||||
|
# convert "from x import y" to "import x.y"
|
||||||
|
# handle "from x import y as z" to "import x.y as z"
|
||||||
|
split_line = line.split()
|
||||||
|
if (line.startswith("from ") and "," not in line and
|
||||||
|
split_line[2] == "import" and split_line[3] != "*" and
|
||||||
|
split_line[1] != "__future__" and
|
||||||
|
(len(split_line) == 4 or
|
||||||
|
(len(split_line) == 6 and split_line[4] == "as"))):
|
||||||
|
mod = split_line[3]
|
||||||
|
return "import %s.%s" % (split_line[1], split_line[3])
|
||||||
|
else:
|
||||||
|
return line
|
||||||
|
|
||||||
|
|
||||||
def nova_todo_format(physical_line):
|
def nova_todo_format(physical_line):
|
||||||
"""
|
"""Check for 'TODO()'.
|
||||||
|
|
||||||
nova HACKING guide recommendation for TODO:
|
nova HACKING guide recommendation for TODO:
|
||||||
Include your name with TODOs as in "#TODO(termie)"
|
Include your name with TODOs as in "#TODO(termie)"
|
||||||
N101
|
N101
|
||||||
@ -61,7 +83,8 @@ def nova_todo_format(physical_line):
|
|||||||
|
|
||||||
|
|
||||||
def nova_except_format(logical_line):
|
def nova_except_format(logical_line):
|
||||||
"""
|
"""Check for 'except:'.
|
||||||
|
|
||||||
nova HACKING guide recommends not using except:
|
nova HACKING guide recommends not using except:
|
||||||
Do not write "except:", use "except Exception:" at the very least
|
Do not write "except:", use "except Exception:" at the very least
|
||||||
N201
|
N201
|
||||||
@ -70,8 +93,9 @@ def nova_except_format(logical_line):
|
|||||||
return 6, "NOVA N201: no 'except:' at least use 'except Exception:'"
|
return 6, "NOVA N201: no 'except:' at least use 'except Exception:'"
|
||||||
|
|
||||||
|
|
||||||
def nova_except_format(logical_line):
|
def nova_except_format_assert(logical_line):
|
||||||
"""
|
"""Check for 'assertRaises(Exception'.
|
||||||
|
|
||||||
nova HACKING guide recommends not using assertRaises(Exception...):
|
nova HACKING guide recommends not using assertRaises(Exception...):
|
||||||
Do not use overly broad Exception type
|
Do not use overly broad Exception type
|
||||||
N202
|
N202
|
||||||
@ -81,7 +105,8 @@ def nova_except_format(logical_line):
|
|||||||
|
|
||||||
|
|
||||||
def nova_one_import_per_line(logical_line):
|
def nova_one_import_per_line(logical_line):
|
||||||
"""
|
"""Check for import format.
|
||||||
|
|
||||||
nova HACKING guide recommends one import per line:
|
nova HACKING guide recommends one import per line:
|
||||||
Do not import more than one module per line
|
Do not import more than one module per line
|
||||||
|
|
||||||
@ -96,9 +121,12 @@ def nova_one_import_per_line(logical_line):
|
|||||||
not is_import_exception(parts[1]):
|
not is_import_exception(parts[1]):
|
||||||
return pos, "NOVA N301: one import per line"
|
return pos, "NOVA N301: one import per line"
|
||||||
|
|
||||||
|
_missingImport = set([])
|
||||||
|
|
||||||
|
|
||||||
def nova_import_module_only(logical_line):
|
def nova_import_module_only(logical_line):
|
||||||
"""
|
"""Check for import module only.
|
||||||
|
|
||||||
nova HACKING guide recommends importing only modules:
|
nova HACKING guide recommends importing only modules:
|
||||||
Do not import objects, only modules
|
Do not import objects, only modules
|
||||||
N302 import only modules
|
N302 import only modules
|
||||||
@ -112,11 +140,14 @@ def nova_import_module_only(logical_line):
|
|||||||
"""
|
"""
|
||||||
current_path = os.path.dirname(pep8.current_file)
|
current_path = os.path.dirname(pep8.current_file)
|
||||||
try:
|
try:
|
||||||
|
with warnings.catch_warnings():
|
||||||
|
warnings.simplefilter('ignore', DeprecationWarning)
|
||||||
valid = True
|
valid = True
|
||||||
if parent:
|
if parent:
|
||||||
if is_import_exception(parent):
|
if is_import_exception(parent):
|
||||||
return
|
return
|
||||||
parent_mod = __import__(parent, globals(), locals(), [mod], -1)
|
parent_mod = __import__(parent, globals(), locals(),
|
||||||
|
[mod], -1)
|
||||||
valid = inspect.ismodule(getattr(parent_mod, mod))
|
valid = inspect.ismodule(getattr(parent_mod, mod))
|
||||||
else:
|
else:
|
||||||
__import__(mod, globals(), locals(), [], -1)
|
__import__(mod, globals(), locals(), [], -1)
|
||||||
@ -125,11 +156,12 @@ def nova_import_module_only(logical_line):
|
|||||||
if added:
|
if added:
|
||||||
sys.path.pop()
|
sys.path.pop()
|
||||||
added = False
|
added = False
|
||||||
return logical_line.find(mod), ("NOVA N304: No relative "
|
return logical_line.find(mod), ("NOVA N304: No "
|
||||||
"imports. '%s' is a relative import" % logical_line)
|
"relative imports. '%s' is a relative import"
|
||||||
|
% logical_line)
|
||||||
return logical_line.find(mod), ("NOVA N302: import only "
|
return logical_line.find(mod), ("NOVA N302: import only "
|
||||||
"modules. '%s' does not import a module" % logical_line)
|
"modules. '%s' does not import a module"
|
||||||
|
% logical_line)
|
||||||
|
|
||||||
except (ImportError, NameError) as exc:
|
except (ImportError, NameError) as exc:
|
||||||
if not added:
|
if not added:
|
||||||
@ -137,8 +169,12 @@ def nova_import_module_only(logical_line):
|
|||||||
sys.path.append(current_path)
|
sys.path.append(current_path)
|
||||||
return importModuleCheck(mod, parent, added)
|
return importModuleCheck(mod, parent, added)
|
||||||
else:
|
else:
|
||||||
|
name = logical_line.split()[1]
|
||||||
|
if (name not in _missingImport and name):
|
||||||
|
if VERBOSE_MISSING_IMPORT:
|
||||||
print >> sys.stderr, ("ERROR: import '%s' failed: %s" %
|
print >> sys.stderr, ("ERROR: import '%s' failed: %s" %
|
||||||
(logical_line, exc))
|
(name, exc))
|
||||||
|
_missingImport.add(name)
|
||||||
added = False
|
added = False
|
||||||
sys.path.pop()
|
sys.path.pop()
|
||||||
return
|
return
|
||||||
@ -148,28 +184,84 @@ def nova_import_module_only(logical_line):
|
|||||||
return logical_line.find(mod), ("NOVA N303: Invalid import, "
|
return logical_line.find(mod), ("NOVA N303: Invalid import, "
|
||||||
"AttributeError raised")
|
"AttributeError raised")
|
||||||
|
|
||||||
|
# convert "from x import y" to " import x.y"
|
||||||
|
# convert "from x import y as z" to " import x.y"
|
||||||
|
import_normalize(logical_line)
|
||||||
split_line = logical_line.split()
|
split_line = logical_line.split()
|
||||||
|
|
||||||
# handle "import x"
|
|
||||||
# handle "import x as y"
|
|
||||||
if (logical_line.startswith("import ") and "," not in logical_line and
|
if (logical_line.startswith("import ") and "," not in logical_line and
|
||||||
(len(split_line) == 2 or
|
(len(split_line) == 2 or
|
||||||
(len(split_line) == 4 and split_line[2] == "as"))):
|
(len(split_line) == 4 and split_line[2] == "as"))):
|
||||||
mod = split_line[1]
|
mod = split_line[1]
|
||||||
return importModuleCheck(mod)
|
return importModuleCheck(mod)
|
||||||
|
|
||||||
# handle "from x import y"
|
|
||||||
# handle "from x import y as z"
|
|
||||||
elif (logical_line.startswith("from ") and "," not in logical_line and
|
|
||||||
split_line[2] == "import" and split_line[3] != "*" and
|
|
||||||
split_line[1] != "__future__" and
|
|
||||||
(len(split_line) == 4 or
|
|
||||||
(len(split_line) == 6 and split_line[4] == "as"))):
|
|
||||||
mod = split_line[3]
|
|
||||||
return importModuleCheck(mod, split_line[1])
|
|
||||||
|
|
||||||
# TODO(jogo) handle "from x import *"
|
# TODO(jogo) handle "from x import *"
|
||||||
|
|
||||||
|
#TODO(jogo): import template: N305
|
||||||
|
|
||||||
|
|
||||||
|
def nova_import_alphabetical(physical_line, line_number, lines):
|
||||||
|
"""Check for imports in alphabetical order.
|
||||||
|
|
||||||
|
nova HACKING guide recommendation for imports:
|
||||||
|
imports in human alphabetical order
|
||||||
|
N306
|
||||||
|
"""
|
||||||
|
# handle import x
|
||||||
|
# use .lower since capitalization shouldn't dictate order
|
||||||
|
split_line = import_normalize(physical_line.strip()).lower().split()
|
||||||
|
split_previous = import_normalize(lines[line_number - 2]
|
||||||
|
).strip().lower().split()
|
||||||
|
# with or without "as y"
|
||||||
|
length = [2, 4]
|
||||||
|
if (len(split_line) in length and len(split_previous) in length and
|
||||||
|
split_line[0] == "import" and split_previous[0] == "import"):
|
||||||
|
if split_line[1] < split_previous[1]:
|
||||||
|
return (0, "NOVA N306: imports not in alphabetical order (%s, %s)"
|
||||||
|
% (split_previous[1], split_line[1]))
|
||||||
|
|
||||||
|
|
||||||
|
def nova_docstring_start_space(physical_line):
|
||||||
|
"""Check for docstring not start with space.
|
||||||
|
|
||||||
|
nova HACKING guide recommendation for docstring:
|
||||||
|
Docstring should not start with space
|
||||||
|
N401
|
||||||
|
"""
|
||||||
|
pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start
|
||||||
|
if (pos != -1 and len(physical_line) > pos + 1):
|
||||||
|
if (physical_line[pos + 3] == ' '):
|
||||||
|
return (pos, "NOVA N401: one line docstring should not start with"
|
||||||
|
" a space")
|
||||||
|
|
||||||
|
|
||||||
|
def nova_docstring_one_line(physical_line):
|
||||||
|
"""Check one line docstring end.
|
||||||
|
|
||||||
|
nova HACKING guide recommendation for one line docstring:
|
||||||
|
A one line docstring looks like this and ends in a period.
|
||||||
|
N402
|
||||||
|
"""
|
||||||
|
pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start
|
||||||
|
end = max([physical_line[-4:-1] == i for i in DOCSTRING_TRIPLE]) # end
|
||||||
|
if (pos != -1 and end and len(physical_line) > pos + 4):
|
||||||
|
if (physical_line[-5] != '.'):
|
||||||
|
return pos, "NOVA N402: one line docstring needs a period"
|
||||||
|
|
||||||
|
|
||||||
|
def nova_docstring_multiline_end(physical_line):
|
||||||
|
"""Check multi line docstring end.
|
||||||
|
|
||||||
|
nova HACKING guide recommendation for docstring:
|
||||||
|
Docstring should end on a new line
|
||||||
|
N403
|
||||||
|
"""
|
||||||
|
pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start
|
||||||
|
if (pos != -1 and len(physical_line) == pos):
|
||||||
|
print physical_line
|
||||||
|
if (physical_line[pos + 3] == ' '):
|
||||||
|
return (pos, "NOVA N403: multi line docstring end on new line")
|
||||||
|
|
||||||
|
|
||||||
FORMAT_RE = re.compile("%(?:"
|
FORMAT_RE = re.compile("%(?:"
|
||||||
"%|" # Ignore plain percents
|
"%|" # Ignore plain percents
|
||||||
@ -264,15 +356,14 @@ current_file = ""
|
|||||||
|
|
||||||
|
|
||||||
def readlines(filename):
|
def readlines(filename):
|
||||||
"""
|
"""Record the current file being tested."""
|
||||||
record the current file being tested
|
|
||||||
"""
|
|
||||||
pep8.current_file = filename
|
pep8.current_file = filename
|
||||||
return open(filename).readlines()
|
return open(filename).readlines()
|
||||||
|
|
||||||
|
|
||||||
def add_nova():
|
def add_nova():
|
||||||
"""
|
"""Monkey patch in nova guidelines.
|
||||||
|
|
||||||
Look for functions that start with nova_ and have arguments
|
Look for functions that start with nova_ and have arguments
|
||||||
and add them to pep8 module
|
and add them to pep8 module
|
||||||
Assumes you know how to write pep8.py checks
|
Assumes you know how to write pep8.py checks
|
||||||
@ -292,4 +383,10 @@ if __name__ == "__main__":
|
|||||||
add_nova()
|
add_nova()
|
||||||
pep8.current_file = current_file
|
pep8.current_file = current_file
|
||||||
pep8.readlines = readlines
|
pep8.readlines = readlines
|
||||||
|
try:
|
||||||
pep8._main()
|
pep8._main()
|
||||||
|
except SystemExit:
|
||||||
|
if len(_missingImport) > 0:
|
||||||
|
print >> sys.stderr, ("%i Missing imports in this test environment"
|
||||||
|
% len(_missingImport))
|
||||||
|
raise
|
||||||
|
Loading…
Reference in New Issue
Block a user