Use flake8 and hacking.
Fixes bug 1172444 Change-Id: I46b733bd16fa49c58c642348988233064d2b673b
This commit is contained in:
Monty Taylor
parent
381049bc03
commit
c23f620f2c
@ -1,14 +1,14 @@
|
||||
# Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
# Copyright (c) 2013 Hewlett-Packard Development Company, L.P.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
|
||||
# implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
@ -565,7 +565,7 @@ port_speed!N/A
|
||||
return self._errors['CMMVC5701E']
|
||||
vol_name = kwargs['obj'].strip('\'\'')
|
||||
|
||||
if not vol_name in self._volumes_list:
|
||||
if vol_name not in self._volumes_list:
|
||||
return self._errors['CMMVC5753E']
|
||||
|
||||
if not force:
|
||||
@ -870,10 +870,10 @@ port_speed!N/A
|
||||
return self._errors['CMMVC5707E']
|
||||
mapping_info['vol'] = kwargs['obj'].strip('\'\'')
|
||||
|
||||
if not mapping_info['vol'] in self._volumes_list:
|
||||
if mapping_info['vol'] not in self._volumes_list:
|
||||
return self._errors['CMMVC5753E']
|
||||
|
||||
if not mapping_info['host'] in self._hosts_list:
|
||||
if mapping_info['host'] not in self._hosts_list:
|
||||
return self._errors['CMMVC5754E']
|
||||
|
||||
if mapping_info['vol'] in self._mappings_list:
|
||||
@ -898,7 +898,7 @@ port_speed!N/A
|
||||
return self._errors['CMMVC5701E']
|
||||
vol = kwargs['obj'].strip('\'\'')
|
||||
|
||||
if not vol in self._mappings_list:
|
||||
if vol not in self._mappings_list:
|
||||
return self._errors['CMMVC5753E']
|
||||
|
||||
if self._mappings_list[vol]['host'] != host:
|
||||
@ -939,13 +939,13 @@ port_speed!N/A
|
||||
if 'source' not in kwargs:
|
||||
return self._errors['CMMVC5707E']
|
||||
source = kwargs['source'].strip('\'\'')
|
||||
if not source in self._volumes_list:
|
||||
if source not in self._volumes_list:
|
||||
return self._errors['CMMVC5754E']
|
||||
|
||||
if 'target' not in kwargs:
|
||||
return self._errors['CMMVC5707E']
|
||||
target = kwargs['target'].strip('\'\'')
|
||||
if not target in self._volumes_list:
|
||||
if target not in self._volumes_list:
|
||||
return self._errors['CMMVC5754E']
|
||||
|
||||
if source == target:
|
||||
|
||||
@ -1420,7 +1420,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
|
||||
'also be set (not equal to -1)'))
|
||||
|
||||
# Check that the requested protocol is enabled
|
||||
if not opts['protocol'] in self._enabled_protocols:
|
||||
if opts['protocol'] not in self._enabled_protocols:
|
||||
raise exception.InvalidInput(
|
||||
reason=_('Illegal value %(prot)s specified for '
|
||||
'storwize_svc_connection_protocol: '
|
||||
|
||||
@ -1,24 +0,0 @@
|
||||
"""
|
||||
wrapper for pyflakes to ignore gettext based warning:
|
||||
"undefined name '_'"
|
||||
|
||||
Synced in from openstack-common
|
||||
"""
|
||||
|
||||
__all__ = ['main']
|
||||
|
||||
import __builtin__ as builtins
|
||||
import sys
|
||||
|
||||
import pyflakes.api
|
||||
from pyflakes import checker
|
||||
|
||||
|
||||
def main():
|
||||
checker.Checker.builtIns = (set(dir(builtins)) |
|
||||
set(['_']) |
|
||||
set(checker._MAGIC_GLOBALS))
|
||||
sys.exit(pyflakes.api.main())
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
405
tools/hacking.py
405
tools/hacking.py
@ -1,405 +0,0 @@
|
||||
#!/usr/bin/env python
|
||||
# vim: tabstop=4 shiftwidth=4 softtabstop=4
|
||||
|
||||
# Copyright (c) 2012, Cloudscaling
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
"""cinder HACKING file compliance testing
|
||||
|
||||
built on top of pep8.py
|
||||
"""
|
||||
|
||||
import inspect
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
import tokenize
|
||||
import warnings
|
||||
|
||||
import pep8
|
||||
|
||||
# Don't need this for testing
|
||||
logging.disable('LOG')
|
||||
|
||||
#N1xx comments
|
||||
#N2xx except
|
||||
#N3xx imports
|
||||
#N4xx docstrings
|
||||
#N5xx dictionaries/lists
|
||||
#N6xx Calling methods
|
||||
#N7xx localization
|
||||
|
||||
IMPORT_EXCEPTIONS = ['sqlalchemy', 'migrate', 'cinder.db.sqlalchemy.session']
|
||||
DOCSTRING_TRIPLE = ['"""', "'''"]
|
||||
VERBOSE_MISSING_IMPORT = False
|
||||
|
||||
|
||||
def is_import_exception(mod):
|
||||
return mod in IMPORT_EXCEPTIONS or \
|
||||
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"))):
|
||||
return "import %s.%s" % (split_line[1], split_line[3])
|
||||
else:
|
||||
return line
|
||||
|
||||
|
||||
def cinder_todo_format(physical_line):
|
||||
"""Check for 'TODO()'.
|
||||
|
||||
cinder HACKING guide recommendation for TODO:
|
||||
Include your name with TODOs as in "#TODO(termie)"
|
||||
N101
|
||||
"""
|
||||
pos = physical_line.find('TODO')
|
||||
pos1 = physical_line.find('TODO(')
|
||||
pos2 = physical_line.find('#') # make sure its a comment
|
||||
if (pos != pos1 and pos2 >= 0 and pos2 < pos):
|
||||
return pos, "CINDER N101: Use TODO(NAME)"
|
||||
|
||||
|
||||
def cinder_except_format(logical_line):
|
||||
"""Check for 'except:'.
|
||||
|
||||
cinder HACKING guide recommends not using except:
|
||||
Do not write "except:", use "except Exception:" at the very least
|
||||
N201
|
||||
"""
|
||||
if logical_line.startswith("except:"):
|
||||
yield 6, "CINDER N201: no 'except:' at least use 'except Exception:'"
|
||||
|
||||
|
||||
def cinder_except_format_assert(logical_line):
|
||||
"""Check for 'assertRaises(Exception'.
|
||||
|
||||
cinder HACKING guide recommends not using assertRaises(Exception...):
|
||||
Do not use overly broad Exception type
|
||||
N202
|
||||
"""
|
||||
if logical_line.startswith("self.assertRaises(Exception"):
|
||||
yield 1, "CINDER N202: assertRaises Exception too broad"
|
||||
|
||||
|
||||
def cinder_one_import_per_line(logical_line):
|
||||
"""Check for import format.
|
||||
|
||||
cinder HACKING guide recommends one import per line:
|
||||
Do not import more than one module per line
|
||||
|
||||
Examples:
|
||||
BAD: from cinder.rpc.common import RemoteError, LOG
|
||||
N301
|
||||
"""
|
||||
pos = logical_line.find(',')
|
||||
parts = logical_line.split()
|
||||
if (pos > -1 and (parts[0] == "import" or
|
||||
parts[0] == "from" and parts[2] == "import") and
|
||||
not is_import_exception(parts[1])):
|
||||
yield pos, "CINDER N301: one import per line"
|
||||
|
||||
_missingImport = set([])
|
||||
|
||||
|
||||
def cinder_import_module_only(logical_line):
|
||||
"""Check for import module only.
|
||||
|
||||
cinder HACKING guide recommends importing only modules:
|
||||
Do not import objects, only modules
|
||||
N302 import only modules
|
||||
N303 Invalid Import
|
||||
N304 Relative Import
|
||||
"""
|
||||
def importModuleCheck(mod, parent=None, added=False):
|
||||
"""
|
||||
If can't find module on first try, recursively check for relative
|
||||
imports
|
||||
"""
|
||||
current_path = os.path.dirname(pep8.current_file)
|
||||
try:
|
||||
with warnings.catch_warnings():
|
||||
warnings.simplefilter('ignore', DeprecationWarning)
|
||||
valid = True
|
||||
if parent:
|
||||
if is_import_exception(parent):
|
||||
return
|
||||
parent_mod = __import__(parent,
|
||||
globals(),
|
||||
locals(),
|
||||
[mod],
|
||||
-1)
|
||||
valid = inspect.ismodule(getattr(parent_mod, mod))
|
||||
else:
|
||||
__import__(mod, globals(), locals(), [], -1)
|
||||
valid = inspect.ismodule(sys.modules[mod])
|
||||
if not valid:
|
||||
if added:
|
||||
sys.path.pop()
|
||||
added = False
|
||||
return (logical_line.find(mod),
|
||||
("CINDER N304: No "
|
||||
"relative imports. '%s' is a relative import"
|
||||
% logical_line))
|
||||
return (logical_line.find(mod),
|
||||
("CINDER N302: import only "
|
||||
"modules. '%s' does not import a module"
|
||||
% logical_line))
|
||||
|
||||
except (ImportError, NameError) as exc:
|
||||
if not added:
|
||||
added = True
|
||||
sys.path.append(current_path)
|
||||
return importModuleCheck(mod, parent, added)
|
||||
else:
|
||||
name = logical_line.split()[1]
|
||||
if name not in _missingImport:
|
||||
if VERBOSE_MISSING_IMPORT:
|
||||
print >> sys.stderr, ("ERROR: import '%s' failed: %s" %
|
||||
(name, exc))
|
||||
_missingImport.add(name)
|
||||
added = False
|
||||
sys.path.pop()
|
||||
return
|
||||
|
||||
except AttributeError:
|
||||
# Invalid import
|
||||
return logical_line.find(mod), ("CINDER N303: Invalid import, "
|
||||
"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()
|
||||
|
||||
if (logical_line.startswith("import ") and
|
||||
"," not in logical_line and
|
||||
(len(split_line) == 2 or
|
||||
(len(split_line) == 4 and split_line[2] == "as"))):
|
||||
mod = split_line[1]
|
||||
rval = importModuleCheck(mod)
|
||||
if rval is not None:
|
||||
yield rval
|
||||
|
||||
# TODO(jogo) handle "from x import *"
|
||||
|
||||
#TODO(jogo): import template: N305
|
||||
|
||||
|
||||
def cinder_import_alphabetical(physical_line, line_number, lines):
|
||||
"""Check for imports in alphabetical order.
|
||||
|
||||
cinder 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,
|
||||
"CINDER N306: imports not in alphabetical order (%s, %s)"
|
||||
% (split_previous[1], split_line[1]))
|
||||
|
||||
|
||||
def cinder_docstring_start_space(physical_line):
|
||||
"""Check for docstring not start with space.
|
||||
|
||||
cinder 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,
|
||||
"CINDER N401: one line docstring should not start with"
|
||||
" a space")
|
||||
|
||||
|
||||
def cinder_docstring_one_line(physical_line):
|
||||
"""Check one line docstring end.
|
||||
|
||||
cinder 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] != '.' and physical_line):
|
||||
return pos, "CINDER N402: one line docstring needs a period"
|
||||
|
||||
|
||||
def cinder_docstring_multiline_end(physical_line):
|
||||
"""Check multi line docstring end.
|
||||
|
||||
cinder 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, "CINDER N403: multi line docstring end on new line")
|
||||
|
||||
|
||||
FORMAT_RE = re.compile("%(?:"
|
||||
"%|" # Ignore plain percents
|
||||
"(\(\w+\))?" # mapping key
|
||||
"([#0 +-]?" # flag
|
||||
"(?:\d+|\*)?" # width
|
||||
"(?:\.\d+)?" # precision
|
||||
"[hlL]?" # length mod
|
||||
"\w))") # type
|
||||
|
||||
|
||||
class LocalizationError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
def check_l18n():
|
||||
"""Generator that checks token stream for localization errors.
|
||||
|
||||
Expects tokens to be ``send``ed one by one.
|
||||
Raises LocalizationError if some error is found.
|
||||
"""
|
||||
while True:
|
||||
try:
|
||||
token_type, text, _, _, _ = yield
|
||||
except GeneratorExit:
|
||||
return
|
||||
if token_type == tokenize.NAME and text == "_":
|
||||
while True:
|
||||
token_type, text, start, _, _ = yield
|
||||
if token_type != tokenize.NL:
|
||||
break
|
||||
if token_type != tokenize.OP or text != "(":
|
||||
continue # not a localization call
|
||||
|
||||
format_string = ''
|
||||
while True:
|
||||
token_type, text, start, _, _ = yield
|
||||
if token_type == tokenize.STRING:
|
||||
format_string += eval(text)
|
||||
elif token_type == tokenize.NL:
|
||||
pass
|
||||
else:
|
||||
break
|
||||
|
||||
if not format_string:
|
||||
raise LocalizationError(
|
||||
start,
|
||||
"CINDER N701: Empty localization string")
|
||||
if token_type != tokenize.OP:
|
||||
raise LocalizationError(
|
||||
start,
|
||||
"CINDER N701: Invalid localization call")
|
||||
if text != ")":
|
||||
if text == "%":
|
||||
raise LocalizationError(
|
||||
start,
|
||||
"CINDER N702: Formatting operation should be outside"
|
||||
" of localization method call")
|
||||
elif text == "+":
|
||||
raise LocalizationError(
|
||||
start,
|
||||
"CINDER N702: Use bare string concatenation instead"
|
||||
" of +")
|
||||
else:
|
||||
raise LocalizationError(
|
||||
start,
|
||||
"CINDER N702: Argument to _ must be just a string")
|
||||
|
||||
format_specs = FORMAT_RE.findall(format_string)
|
||||
positional_specs = [(key, spec) for key, spec in format_specs
|
||||
if not key and spec]
|
||||
# not spec means %%, key means %(smth)s
|
||||
if len(positional_specs) > 1:
|
||||
raise LocalizationError(
|
||||
start,
|
||||
"CINDER N703: Multiple positional placeholders")
|
||||
|
||||
|
||||
def cinder_localization_strings(logical_line, tokens):
|
||||
"""Check localization in line.
|
||||
|
||||
N701: bad localization call
|
||||
N702: complex expression instead of string as argument to _()
|
||||
N703: multiple positional placeholders
|
||||
"""
|
||||
|
||||
gen = check_l18n()
|
||||
next(gen)
|
||||
try:
|
||||
map(gen.send, tokens)
|
||||
gen.close()
|
||||
except LocalizationError as e:
|
||||
yield e.args
|
||||
|
||||
#TODO(jogo) Dict and list objects
|
||||
|
||||
current_file = ""
|
||||
|
||||
|
||||
def readlines(filename):
|
||||
"""Record the current file being tested."""
|
||||
pep8.current_file = filename
|
||||
return open(filename).readlines()
|
||||
|
||||
|
||||
def add_cinder():
|
||||
"""Monkey patch in cinder guidelines.
|
||||
|
||||
Look for functions that start with cinder_ and have arguments
|
||||
and add them to pep8 module
|
||||
Assumes you know how to write pep8.py checks
|
||||
"""
|
||||
for name, function in globals().items():
|
||||
if not inspect.isfunction(function):
|
||||
continue
|
||||
args = inspect.getargspec(function)[0]
|
||||
if args and name.startswith("cinder"):
|
||||
exec("pep8.%s = %s" % (name, name))
|
||||
|
||||
if __name__ == "__main__":
|
||||
#include cinder path
|
||||
sys.path.append(os.getcwd())
|
||||
#CINDER error codes start with an N
|
||||
pep8.ERRORCODE_REGEX = re.compile(r'[EWN]\d{3}')
|
||||
add_cinder()
|
||||
pep8.current_file = current_file
|
||||
pep8.readlines = readlines
|
||||
try:
|
||||
pep8._main()
|
||||
finally:
|
||||
if len(_missingImport) > 0:
|
||||
print >> sys.stderr, ("%i imports missing in this test environment"
|
||||
% len(_missingImport))
|
||||
@ -1,4 +1,9 @@
|
||||
# This file is managed by openstack-depends
|
||||
# Install bounded pep8/pyflakes first, then let flake8 install
|
||||
pep8==1.4.5
|
||||
pyflakes==0.7.2
|
||||
flake8==2.0
|
||||
hacking>=0.5.3,<0.6
|
||||
|
||||
coverage>=3.6
|
||||
distribute>=0.6.24
|
||||
hp3parclient>=1.0.0
|
||||
@ -9,8 +14,5 @@ nose
|
||||
nosehtmloutput>=0.0.3
|
||||
nosexcover
|
||||
openstack.nose_plugin>=0.7
|
||||
pep8==1.3.3
|
||||
psycopg2
|
||||
pylint==0.25.2
|
||||
setuptools_git>=0.4
|
||||
sphinx>=1.1.2
|
||||
|
||||
15
tox.ini
15
tox.ini
@ -21,10 +21,8 @@ commands =
|
||||
|
||||
[testenv:pep8]
|
||||
commands =
|
||||
python tools/hacking.py --ignore=N4,E125,E126,E711,E712 --repeat --show-source \
|
||||
--exclude=.venv,.tox,dist,doc,openstack,*egg .
|
||||
python tools/hacking.py --ignore=N4,E125,E126,E711,E712 --repeat --show-source \
|
||||
--filename=cinder* bin
|
||||
flake8
|
||||
flake8 --filename=cinder* bin
|
||||
|
||||
[testenv:venv]
|
||||
commands = {posargs}
|
||||
@ -32,12 +30,13 @@ commands = {posargs}
|
||||
[testenv:cover]
|
||||
setenv = NOSE_WITH_COVERAGE=1
|
||||
|
||||
[testenv:pyflakes]
|
||||
deps = pyflakes
|
||||
commands = python tools/flakes.py cinder
|
||||
|
||||
[testenv:pylint]
|
||||
setenv = VIRTUAL_ENV={envdir}
|
||||
deps = -r{toxinidir}/tools/pip-requires
|
||||
pylint==0.26.0
|
||||
commands = bash tools/lintstack.sh
|
||||
|
||||
[flake8]
|
||||
ignore = E12,E711,E712,H3,H4,F
|
||||
builtins = _
|
||||
exclude = .venv,.tox,dist,doc,openstack,*egg
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user